* cap mask differences for `Client::stat` vs `Client::fstat`
@ 2016-04-13 17:26 Noah Watkins
2016-04-13 20:24 ` Gregory Farnum
2016-04-14 1:49 ` Yan, Zheng
0 siblings, 2 replies; 7+ messages in thread
From: Noah Watkins @ 2016-04-13 17:26 UTC (permalink / raw)
To: ceph-devel
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?
Thanks,
Noah
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cap mask differences for `Client::stat` vs `Client::fstat`
2016-04-13 17:26 cap mask differences for `Client::stat` vs `Client::fstat` Noah Watkins
@ 2016-04-13 20:24 ` Gregory Farnum
2016-04-14 1:49 ` Yan, Zheng
1 sibling, 0 replies; 7+ messages in thread
From: Gregory Farnum @ 2016-04-13 20:24 UTC (permalink / raw)
To: Noah Watkins; +Cc: ceph-devel
On Wed, Apr 13, 2016 at 10:26 AM, Noah Watkins <noahwatkins@gmail.com> 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?
Based off of "man fstat" and looking at the git history, I think this
is an accident. I certainly can't think of any reason why we'd need Fb
(for instance) in order to answer a stat request. Send in a PR? :)
-Greg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cap mask differences for `Client::stat` vs `Client::fstat`
2016-04-13 17:26 cap mask differences for `Client::stat` vs `Client::fstat` Noah Watkins
2016-04-13 20:24 ` Gregory Farnum
@ 2016-04-14 1:49 ` Yan, Zheng
2016-04-23 23:48 ` Noah Watkins
1 sibling, 1 reply; 7+ messages in thread
From: Yan, Zheng @ 2016-04-14 1:49 UTC (permalink / raw)
To: Noah Watkins; +Cc: ceph-devel
On Thu, Apr 14, 2016 at 1:26 AM, Noah Watkins <noahwatkins@gmail.com> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cap mask differences for `Client::stat` vs `Client::fstat`
2016-04-14 1:49 ` Yan, Zheng
@ 2016-04-23 23:48 ` Noah Watkins
2016-04-25 1:48 ` Yan, Zheng
2016-04-26 19:41 ` Gregory Farnum
0 siblings, 2 replies; 7+ messages in thread
From: Noah Watkins @ 2016-04-23 23:48 UTC (permalink / raw)
To: Yan, Zheng, Gregory Farnum; +Cc: ceph-devel
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.
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@gmail.com> wrote:
> On Thu, Apr 14, 2016 at 1:26 AM, Noah Watkins <noahwatkins@gmail.com> 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@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: cap mask differences for `Client::stat` vs `Client::fstat`
2016-04-23 23:48 ` Noah Watkins
@ 2016-04-25 1:48 ` Yan, Zheng
2016-04-26 19:41 ` Gregory Farnum
1 sibling, 0 replies; 7+ messages in thread
From: Yan, Zheng @ 2016-04-25 1:48 UTC (permalink / raw)
To: Noah Watkins; +Cc: Gregory Farnum, ceph-devel
On Sun, Apr 24, 2016 at 7:48 AM, Noah Watkins <noahwatkins@gmail.com> 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.
>
> 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);
Looks good. please open a RP.
regards
Yan, Zheng
>
> On Wed, Apr 13, 2016 at 6:49 PM, Yan, Zheng <ukernel@gmail.com> wrote:
>> On Thu, Apr 14, 2016 at 1:26 AM, Noah Watkins <noahwatkins@gmail.com> 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@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cap mask differences for `Client::stat` vs `Client::fstat`
2016-04-23 23:48 ` Noah Watkins
2016-04-25 1:48 ` Yan, Zheng
@ 2016-04-26 19:41 ` Gregory Farnum
2016-05-04 4:47 ` Noah Watkins
1 sibling, 1 reply; 7+ messages in thread
From: Gregory Farnum @ 2016-04-26 19:41 UTC (permalink / raw)
To: Noah Watkins; +Cc: Yan, Zheng, ceph-devel
On Sat, Apr 23, 2016 at 6:48 PM, Noah Watkins <noahwatkins@gmail.com> 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@gmail.com> wrote:
>> On Thu, Apr 14, 2016 at 1:26 AM, Noah Watkins <noahwatkins@gmail.com> 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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cap mask differences for `Client::stat` vs `Client::fstat`
2016-04-26 19:41 ` Gregory Farnum
@ 2016-05-04 4:47 ` Noah Watkins
0 siblings, 0 replies; 7+ messages in thread
From: Noah Watkins @ 2016-05-04 4:47 UTC (permalink / raw)
To: Gregory Farnum; +Cc: Yan, Zheng, ceph-devel
http://tracker.ceph.com/issues/15723
On Tue, Apr 26, 2016 at 12:41 PM, Gregory Farnum <gfarnum@redhat.com> wrote:
> On Sat, Apr 23, 2016 at 6:48 PM, Noah Watkins <noahwatkins@gmail.com> 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@gmail.com> wrote:
>>> On Thu, Apr 14, 2016 at 1:26 AM, Noah Watkins <noahwatkins@gmail.com> 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@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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-04 4:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-13 17:26 cap mask differences for `Client::stat` vs `Client::fstat` Noah Watkins
2016-04-13 20:24 ` Gregory Farnum
2016-04-14 1:49 ` Yan, Zheng
2016-04-23 23:48 ` Noah Watkins
2016-04-25 1:48 ` Yan, Zheng
2016-04-26 19:41 ` Gregory Farnum
2016-05-04 4:47 ` Noah Watkins
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.