| Message ID | 2073248911.84014.1518250545543@mail.yahoo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
(cut down CC list to ceph-devel) If you skip taking locks, then you will be serving the client stale metadata, and breaking the ordinary consistency rules for POSIX file IO. That might be what you want if you have a special application and you want a specially hacked MDS to run it faster, but if you are running an application that expects a filesystem to behave correctly then it would be hard to predict the ways in which it could break. John On Sat, Feb 10, 2018 at 8:15 AM, Mark Meyers <MarkMeyers.MMY@gmail.com> wrote: > I found ls command costs extremely long time (form 30s to 3mins). > So I tried to remove acquire_locks in mds's getattr handler: handle_clinet_getattr. > Everything looks just good: > First, the size of files get updated in time. > Second, the read and write to file from clinets works ok. > > So, please, can I just omiit acquire_locks in mds getattr handler?(if client request doesn't requier or have Fwb caps) > > Code: > > diff -- git a/src/mds/Server.cc n/src/mds/Server.cc > > --- a/src/mds/Server.cc > +++ b/src/mds/Server.cc > @@ -3021,7 +3021,10 @@ void Server::handler_client_getattr(MDRequestRef& mdr, bool is_lookup) > if ((mask & CEPH_CAP_FILE_SHARED) && (issued & CEPH_CAP_FILE_EXCL) == 0) rdlocks.insert(&ref->filelock); > if ((mask & CEPH_CAP_XATTR_SHARED) && (issued & CEPH_CAP_XATTR_EXCL) == 0) rdlocks.insert(&ref->xattrlock); > - if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks)) > + dout(20) << __func__ << " mask " << ccap_string(mask) << " issued " << > ccap_string(issued) << " " << !is_lookup << " ." << dendl; > + if(!(mask & CEPH_CAP_FILE_WR) && !(mask & CEPH_CAP_FILE_BUFFER) && !(issued & CEPH_CAP_FILE_WR) && !(iissued & CEPH_CAP_FILE_BUFFER) && !is_lookup) > + dout(10) << "no Fw/b in mask " << ccap_string(mask) << " or issued " << ccap_string(issued) << " " << !is_lookup << " ." << dendl;+ else if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks)) > return; > > > if (!check_access(mdr, ref, MAY_READ)) > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Even more than that, the locks also control metadata trimming and things. You might actually crash the server, not "merely" get inconsistent results back! -Greg On Sat, Feb 10, 2018 at 9:53 AM, John Spray <jspray@redhat.com> wrote: > (cut down CC list to ceph-devel) > > If you skip taking locks, then you will be serving the client stale > metadata, and breaking the ordinary consistency rules for POSIX file > IO. That might be what you want if you have a special application and > you want a specially hacked MDS to run it faster, but if you are > running an application that expects a filesystem to behave correctly > then it would be hard to predict the ways in which it could break. > > John > > On Sat, Feb 10, 2018 at 8:15 AM, Mark Meyers <MarkMeyers.MMY@gmail.com> wrote: >> I found ls command costs extremely long time (form 30s to 3mins). >> So I tried to remove acquire_locks in mds's getattr handler: handle_clinet_getattr. >> Everything looks just good: >> First, the size of files get updated in time. >> Second, the read and write to file from clinets works ok. >> >> So, please, can I just omiit acquire_locks in mds getattr handler?(if client request doesn't requier or have Fwb caps) >> >> Code: >> >> diff -- git a/src/mds/Server.cc n/src/mds/Server.cc >> >> --- a/src/mds/Server.cc >> +++ b/src/mds/Server.cc >> @@ -3021,7 +3021,10 @@ void Server::handler_client_getattr(MDRequestRef& mdr, bool is_lookup) >> if ((mask & CEPH_CAP_FILE_SHARED) && (issued & CEPH_CAP_FILE_EXCL) == 0) rdlocks.insert(&ref->filelock); >> if ((mask & CEPH_CAP_XATTR_SHARED) && (issued & CEPH_CAP_XATTR_EXCL) == 0) rdlocks.insert(&ref->xattrlock); >> - if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks)) >> + dout(20) << __func__ << " mask " << ccap_string(mask) << " issued " << >> ccap_string(issued) << " " << !is_lookup << " ." << dendl; >> + if(!(mask & CEPH_CAP_FILE_WR) && !(mask & CEPH_CAP_FILE_BUFFER) && !(issued & CEPH_CAP_FILE_WR) && !(iissued & CEPH_CAP_FILE_BUFFER) && !is_lookup) >> + dout(10) << "no Fw/b in mask " << ccap_string(mask) << " or issued " << ccap_string(issued) << " " << !is_lookup << " ." << dendl;+ else if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks)) >> return; >> >> >> if (!check_access(mdr, ref, MAY_READ)) >> -- >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -- git a/src/mds/Server.cc n/src/mds/Server.cc --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -3021,7 +3021,10 @@ void Server::handler_client_getattr(MDRequestRef& mdr, bool is_lookup) if ((mask & CEPH_CAP_FILE_SHARED) && (issued & CEPH_CAP_FILE_EXCL) == 0) rdlocks.insert(&ref->filelock); if ((mask & CEPH_CAP_XATTR_SHARED) && (issued & CEPH_CAP_XATTR_EXCL) == 0) rdlocks.insert(&ref->xattrlock); - if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks)) + dout(20) << __func__ << " mask " << ccap_string(mask) << " issued " << ccap_string(issued) << " " << !is_lookup << " ." << dendl; + if(!(mask & CEPH_CAP_FILE_WR) && !(mask & CEPH_CAP_FILE_BUFFER) && !(issued & CEPH_CAP_FILE_WR) && !(iissued & CEPH_CAP_FILE_BUFFER) && !is_lookup) + dout(10) << "no Fw/b in mask " << ccap_string(mask) << " or issued " << ccap_string(issued) << " " << !is_lookup << " ." << dendl;+ else if (!mds->locker->acquire_locks(mdr, rdlocks, wrlocks, xlocks)) return; if (!check_access(mdr, ref, MAY_READ))