On Tue, Jun 26, 2018 at 10:41 AM cgxu519 <cgxu519@xxxxxxx> wrote:
>
> On 06/26/2018 04:18 PM, Ilya Dryomov wrote:
> > On Tue, Jun 26, 2018 at 4:09 AM cgxu519 <cgxu519@xxxxxxx> wrote:
> >>
> >>
> >> On 06/24/2018 05:05 PM, Ilya Dryomov wrote:
> >>> On Sun, Jun 24, 2018 at 10:57 AM cgxu519 <cgxu519@xxxxxxx> wrote:
> >>>> On 06/24/2018 04:49 PM, Ilya Dryomov wrote:
> >>>>> On Sat, Jun 23, 2018 at 2:55 PM Chengguang Xu <cgxu519@xxxxxxx> wrote:
> >>>>>> Though val_size1/val_size2 are both type sizt_t but
> >>>>>> ceph_pagelist_encode_string() can only handle string
> >>>>>> size max to U32_MAX, so change hardcode 8 to sizeof(u32).
> >>>>>>
> >>>>>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxx>
> >>>>>> ---
> >>>>>> fs/ceph/acl.c | 6 ++++--
> >>>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
> >>>>>> index 3351ea14390b..f736108604a6 100644
> >>>>>> --- a/fs/ceph/acl.c
> >>>>>> +++ b/fs/ceph/acl.c
> >>>>>> @@ -219,7 +219,8 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
> >>>>>>
> >>>>>> if (acl) {
> >>>>>> size_t len = strlen(XATTR_NAME_POSIX_ACL_ACCESS);
> >>>>>> - err = ceph_pagelist_reserve(pagelist, len + val_size1 + 8);
> >>>>>> + err = ceph_pagelist_reserve(pagelist, len + val_size1 +
> >>>>>> + sizeof(u32));
> >>>>>> if (err)
> >>>>>> goto out_err;
> >>>>>> ceph_pagelist_encode_string(pagelist, XATTR_NAME_POSIX_ACL_ACCESS,
> >>>>>> @@ -233,7 +234,8 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
> >>>>>> }
> >>>>>> if (default_acl) {
> >>>>>> size_t len = strlen(XATTR_NAME_POSIX_ACL_DEFAULT);
> >>>>>> - err = ceph_pagelist_reserve(pagelist, len + val_size2 + 8);
> >>>>>> + err = ceph_pagelist_reserve(pagelist, len + val_size2 +
> >>>>>> + sizeof(u32));
> >>>>>> if (err)
> >>>>>> goto out_err;
> >>>>>> err = ceph_pagelist_encode_string(pagelist,
> >>>>> This is wrong. In both cases it reserves pages for two string-like
> >>>>> things, str_size + 4 each, for a total of str_size1 + str_size2 + 8.
> >>>> Ah, yes. Could we change to sizeof(u32) + sizeof(u32) instead?
> >>> No, that would be unnecessary churn.
> >> Probably we can change the first reservation size from PAGE_SIZE to
> >> sizeof(u32)
> >> in ceph_pre_init_acls() because actually we encode int type 2 or 1 using
> >> ceph_pagelist_encode_32(). Am I missing something?
> > It wouldn't change anything. It is a fresly created pagelist (i.e.
> > a list of pages). The granularity is a page, so even if you reserve
> > just one byte, it would still allocate a page.
>
> I know what do you mean and I agree actually it changes nothing in
> technique.
> I just hope to make the code a bit more readable here.
Then let's remove that call to ceph_pagelist_reserve() altogether and
check the return value of ceph_pagelist_encode_32() instead.
Thanks,
Ilya
--
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
