Re: cap mask differences for `Client::stat` vs `Client::fstat`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



http://tracker.ceph.com/issues/15723

On Tue, Apr 26, 2016 at 12:41 PM, Gregory Farnum <gfarnum@xxxxxxxxxx> wrote:
> On Sat, Apr 23, 2016 at 6:48 PM, Noah Watkins <noahwatkins@xxxxxxxxx> wrote:
>> I have a patch for this (below), but I see something that doesn't seem
>> quite right.
>>
>> Here is a simple fs client. When its running by itself the fstat/sec
>> rate is high. When a second client copy of this client starts, the
>> first client slows down as expected, but the new client blocks. I
>> would expect both clients to make progress, but until the first client
>> exits, the second client is blocked.
>
> Ugh. Clearly we have a cap release sequence bug — probably the client
> is spinning off new requests instead of blocking them on a cap
> release. Can you create a tracker for it?
> -Greg
>
>>
>> fd = cephfs.open('file-1', 'w', 0755)
>> count = 0
>> start = time.time()
>> while True:
>>     stat = cephfs.fstat(fd)
>>     count += 1
>>     if time.time() > (start + 1):
>>         print(count)
>>         count = 0
>>         start = time.time()
>>
>> - Noah
>>
>> Patch:
>>
>> 709840b - fsclient: fstat can take inode all cap (10 minutes ago) <Noah Watkins>
>> diff --git a/src/client/Client.cc b/src/client/Client.cc
>> index 7fcd907..94e4db5 100644
>> --- a/src/client/Client.cc
>> +++ b/src/client/Client.cc
>> @@ -8668,16 +8668,16 @@ int Client::_fsync(Fh *f, bool syncdataonly)
>>    return _fsync(f->inode.get(), syncdataonly);
>>  }
>>
>> -int Client::fstat(int fd, struct stat *stbuf)
>> +int Client::fstat(int fd, struct stat *stbuf, int mask)
>>  {
>>    Mutex::Locker lock(client_lock);
>> -  tout(cct) << "fstat" << std::endl;
>> +  tout(cct) << "fstat" << " mask " << mask << std::endl;
>>    tout(cct) << fd << std::endl;
>>
>>    Fh *f = get_filehandle(fd);
>>    if (!f)
>>      return -EBADF;
>> -  int r = _getattr(f->inode, -1);
>> +  int r = _getattr(f->inode, mask);
>>    if (r < 0)
>>      return r;
>>    fill_stat(f->inode, stbuf, NULL);
>> diff --git a/src/client/Client.h b/src/client/Client.h
>> index d912db0..860f28b 100644
>> --- a/src/client/Client.h
>> +++ b/src/client/Client.h
>> @@ -981,7 +981,7 @@ public:
>>    int fake_write_size(int fd, loff_t size);
>>    int ftruncate(int fd, loff_t size);
>>    int fsync(int fd, bool syncdataonly);
>> -  int fstat(int fd, struct stat *stbuf);
>> +  int fstat(int fd, struct stat *stbuf, int mask=CEPH_STAT_CAP_INODE_ALL);
>>    int fallocate(int fd, int mode, loff_t offset, loff_t length);
>>
>> On Wed, Apr 13, 2016 at 6:49 PM, Yan, Zheng <ukernel@xxxxxxxxx> wrote:
>>> On Thu, Apr 14, 2016 at 1:26 AM, Noah Watkins <noahwatkins@xxxxxxxxx> wrote:
>>>> In the libcephfs client it looks like both `Client::stat` and
>>>> `Client::lstat` use a default mask of CEPH_STAT_CAP_INODE_ALL and are
>>>> able to return immediately from `Client::_getattr` without making an
>>>> MDS request. However, `Client::fstat` uses hard-coded -1 as the cap
>>>> mask and always issues a MDS request. Is this the intended behavior,
>>>> and if so, what are different semantics of fstat that make this a
>>>> requirement?
>>>
>>> I think it's a bug, Client::fstat should use CEPH_STAT_CAP_INODE_ALL too.
>>>
>>> Regards
>>> Yan, Zheng
>>>
>>>>
>>>> Thanks,
>>>> Noah
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux