* [PATCH] attr: adbjust acl_max of f2fs @ 2017-04-28 13:13 Chao Yu 2017-05-03 7:29 ` Eryu Guan 2017-05-03 17:02 ` Jaegeuk Kim 0 siblings, 2 replies; 6+ messages in thread From: Chao Yu @ 2017-04-28 13:13 UTC (permalink / raw) To: fstests, eguan; +Cc: jaegeuk, linux-f2fs-devel, Chao Yu From: Chao Yu <yuchao0@huawei.com> f2fs has set inline_xattr as a default option, and introduced a new option named 'noinline_xattr' for disabling default inline_xattr option. So in _acl_get_max we need to check 'noinline_xattr' string in fs option, otherwise we may select the wrong max acl number since we always found the string 'ininline_xattr' in fs option. Additionally, f2fs has changed disk layout of xattr block a bit, so will contain one more entry in both inline and noinline xattr inode, this patch will modify the max acl number to adjust it. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- common/attr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/attr b/common/attr index ac139e61..6d4f68ed 100644 --- a/common/attr +++ b/common/attr @@ -43,11 +43,11 @@ _acl_get_max() echo 8191 ;; f2fs) - _fs_options $TEST_DEV | grep "inline_xattr" >/dev/null 2>&1 + _fs_options $TEST_DEV | grep "noinline_xattr" >/dev/null 2>&1 if [ $? -eq 0 ]; then - echo 531 + echo 507 else - echo 506 + echo 532 fi ;; *) -- 2.12.2.575.gb14f27f ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] attr: adbjust acl_max of f2fs 2017-04-28 13:13 [PATCH] attr: adbjust acl_max of f2fs Chao Yu @ 2017-05-03 7:29 ` Eryu Guan 2017-05-03 8:07 ` Chao Yu 2017-05-03 17:02 ` Jaegeuk Kim 1 sibling, 1 reply; 6+ messages in thread From: Eryu Guan @ 2017-05-03 7:29 UTC (permalink / raw) To: Chao Yu; +Cc: fstests, jaegeuk, linux-f2fs-devel, Chao Yu On Fri, Apr 28, 2017 at 09:13:07PM +0800, Chao Yu wrote: > From: Chao Yu <yuchao0@huawei.com> > > f2fs has set inline_xattr as a default option, and introduced a new option > named 'noinline_xattr' for disabling default inline_xattr option. So in > _acl_get_max we need to check 'noinline_xattr' string in fs option, > otherwise we may select the wrong max acl number since we always found > the string 'ininline_xattr' in fs option. > > Additionally, f2fs has changed disk layout of xattr block a bit, so will > contain one more entry in both inline and noinline xattr inode, this patch > will modify the max acl number to adjust it. > > Signed-off-by: Chao Yu <yuchao0@huawei.com> Thanks for the patch! It looks fine to me from fstests POV, but need review from f2fs developers too. Thanks, Eryu > --- > common/attr | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/attr b/common/attr > index ac139e61..6d4f68ed 100644 > --- a/common/attr > +++ b/common/attr > @@ -43,11 +43,11 @@ _acl_get_max() > echo 8191 > ;; > f2fs) > - _fs_options $TEST_DEV | grep "inline_xattr" >/dev/null 2>&1 > + _fs_options $TEST_DEV | grep "noinline_xattr" >/dev/null 2>&1 > if [ $? -eq 0 ]; then > - echo 531 > + echo 507 > else > - echo 506 > + echo 532 > fi > ;; > *) > -- > 2.12.2.575.gb14f27f > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] attr: adbjust acl_max of f2fs 2017-05-03 7:29 ` Eryu Guan @ 2017-05-03 8:07 ` Chao Yu 0 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2017-05-03 8:07 UTC (permalink / raw) To: Eryu Guan, jaegeuk; +Cc: Chao Yu, fstests, linux-f2fs-devel On 2017/5/3 15:29, Eryu Guan wrote: > On Fri, Apr 28, 2017 at 09:13:07PM +0800, Chao Yu wrote: >> From: Chao Yu <yuchao0@huawei.com> >> >> f2fs has set inline_xattr as a default option, and introduced a new option >> named 'noinline_xattr' for disabling default inline_xattr option. So in >> _acl_get_max we need to check 'noinline_xattr' string in fs option, >> otherwise we may select the wrong max acl number since we always found >> the string 'ininline_xattr' in fs option. >> >> Additionally, f2fs has changed disk layout of xattr block a bit, so will >> contain one more entry in both inline and noinline xattr inode, this patch >> will modify the max acl number to adjust it. >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> > > Thanks for the patch! It looks fine to me from fstests POV, but need > review from f2fs developers too. Eryu, thanks for the review. ;) To Jaegeuk, could you help to review this patch? Thanks, > > Thanks, > Eryu > >> --- >> common/attr | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/common/attr b/common/attr >> index ac139e61..6d4f68ed 100644 >> --- a/common/attr >> +++ b/common/attr >> @@ -43,11 +43,11 @@ _acl_get_max() >> echo 8191 >> ;; >> f2fs) >> - _fs_options $TEST_DEV | grep "inline_xattr" >/dev/null 2>&1 >> + _fs_options $TEST_DEV | grep "noinline_xattr" >/dev/null 2>&1 >> if [ $? -eq 0 ]; then >> - echo 531 >> + echo 507 >> else >> - echo 506 >> + echo 532 >> fi >> ;; >> *) >> -- >> 2.12.2.575.gb14f27f >> > > . > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] attr: adbjust acl_max of f2fs 2017-04-28 13:13 [PATCH] attr: adbjust acl_max of f2fs Chao Yu 2017-05-03 7:29 ` Eryu Guan @ 2017-05-03 17:02 ` Jaegeuk Kim 2017-05-04 15:32 ` Chao Yu 1 sibling, 1 reply; 6+ messages in thread From: Jaegeuk Kim @ 2017-05-03 17:02 UTC (permalink / raw) To: Chao Yu; +Cc: eguan, fstests, linux-f2fs-devel Hello, On 04/28, Chao Yu wrote: > From: Chao Yu <yuchao0@huawei.com> > > f2fs has set inline_xattr as a default option, and introduced a new option > named 'noinline_xattr' for disabling default inline_xattr option. So in > _acl_get_max we need to check 'noinline_xattr' string in fs option, > otherwise we may select the wrong max acl number since we always found > the string 'ininline_xattr' in fs option. > > Additionally, f2fs has changed disk layout of xattr block a bit, so will > contain one more entry in both inline and noinline xattr inode, this patch > will modify the max acl number to adjust it. > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > common/attr | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/attr b/common/attr > index ac139e61..6d4f68ed 100644 > --- a/common/attr > +++ b/common/attr > @@ -43,11 +43,11 @@ _acl_get_max() > echo 8191 > ;; > f2fs) > - _fs_options $TEST_DEV | grep "inline_xattr" >/dev/null 2>&1 > + _fs_options $TEST_DEV | grep "noinline_xattr" >/dev/null 2>&1 This breaks old kernel support which has no "noinline_xattr". It'd be good to check "inline_xattr" again after checking "noinline_xattr". And, in terms of different number of entries, can we get the number from local.config by adding an export symbol likewise FSTYP? Thanks, > if [ $? -eq 0 ]; then > - echo 531 > + echo 507 > else > - echo 506 > + echo 532 > fi > ;; > *) > -- > 2.12.2.575.gb14f27f ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] attr: adbjust acl_max of f2fs 2017-05-03 17:02 ` Jaegeuk Kim @ 2017-05-04 15:32 ` Chao Yu 2017-05-04 17:39 ` Jaegeuk Kim 0 siblings, 1 reply; 6+ messages in thread From: Chao Yu @ 2017-05-04 15:32 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: eguan, fstests, linux-f2fs-devel Hi Jaegeuk, On 2017/5/4 1:02, Jaegeuk Kim wrote: > Hello, > > On 04/28, Chao Yu wrote: >> From: Chao Yu <yuchao0@huawei.com> >> >> f2fs has set inline_xattr as a default option, and introduced a new option >> named 'noinline_xattr' for disabling default inline_xattr option. So in >> _acl_get_max we need to check 'noinline_xattr' string in fs option, >> otherwise we may select the wrong max acl number since we always found >> the string 'ininline_xattr' in fs option. >> >> Additionally, f2fs has changed disk layout of xattr block a bit, so will >> contain one more entry in both inline and noinline xattr inode, this patch >> will modify the max acl number to adjust it. >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> common/attr | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/common/attr b/common/attr >> index ac139e61..6d4f68ed 100644 >> --- a/common/attr >> +++ b/common/attr >> @@ -43,11 +43,11 @@ _acl_get_max() >> echo 8191 >> ;; >> f2fs) >> - _fs_options $TEST_DEV | grep "inline_xattr" >/dev/null 2>&1 >> + _fs_options $TEST_DEV | grep "noinline_xattr" >/dev/null 2>&1 > > This breaks old kernel support which has no "noinline_xattr". It'd be good to > check "inline_xattr" again after checking "noinline_xattr". And, in terms of Hmm, we have changed xattr layout a bit in new kernel, so even we have knew that user set inline_xattr mount option, still we don't know whether the last kernel was been used, so we can not decide to choose 532 or 531 as acl_max value. Maybe we need to check result of 'uname -r' either. > different number of entries, can we get the number from local.config by adding > an export symbol likewise FSTYP? You mean adding one more configuration parameter which can be defined by user, then _acl_get_max can return correct acl_max value depend on that parameter? Thanks, > > Thanks, > >> if [ $? -eq 0 ]; then >> - echo 531 >> + echo 507 >> else >> - echo 506 >> + echo 532 >> fi >> ;; >> *) >> -- >> 2.12.2.575.gb14f27f ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] attr: adbjust acl_max of f2fs 2017-05-04 15:32 ` Chao Yu @ 2017-05-04 17:39 ` Jaegeuk Kim 0 siblings, 0 replies; 6+ messages in thread From: Jaegeuk Kim @ 2017-05-04 17:39 UTC (permalink / raw) To: Chao Yu; +Cc: fstests, eguan, linux-f2fs-devel, Chao Yu On 05/04, Chao Yu wrote: > Hi Jaegeuk, > > On 2017/5/4 1:02, Jaegeuk Kim wrote: > > Hello, > > > > On 04/28, Chao Yu wrote: > >> From: Chao Yu <yuchao0@huawei.com> > >> > >> f2fs has set inline_xattr as a default option, and introduced a new option > >> named 'noinline_xattr' for disabling default inline_xattr option. So in > >> _acl_get_max we need to check 'noinline_xattr' string in fs option, > >> otherwise we may select the wrong max acl number since we always found > >> the string 'ininline_xattr' in fs option. > >> > >> Additionally, f2fs has changed disk layout of xattr block a bit, so will > >> contain one more entry in both inline and noinline xattr inode, this patch > >> will modify the max acl number to adjust it. > >> > >> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >> --- > >> common/attr | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/common/attr b/common/attr > >> index ac139e61..6d4f68ed 100644 > >> --- a/common/attr > >> +++ b/common/attr > >> @@ -43,11 +43,11 @@ _acl_get_max() > >> echo 8191 > >> ;; > >> f2fs) > >> - _fs_options $TEST_DEV | grep "inline_xattr" >/dev/null 2>&1 > >> + _fs_options $TEST_DEV | grep "noinline_xattr" >/dev/null 2>&1 > > > > This breaks old kernel support which has no "noinline_xattr". It'd be good to > > check "inline_xattr" again after checking "noinline_xattr". And, in terms of > > Hmm, we have changed xattr layout a bit in new kernel, so even we have > knew that user set inline_xattr mount option, still we don't know > whether the last kernel was been used, so we can not decide to choose > 532 or 531 as acl_max value. Maybe we need to check result of 'uname -r' > either. > > > different number of entries, can we get the number from local.config by adding > > an export symbol likewise FSTYP? > > You mean adding one more configuration parameter which can be defined by > user, then _acl_get_max can return correct acl_max value depend on that > parameter? How about this? if noinline_xattr exists acl_max=506 else if inline_xattr exists acl_max=531 else /* for old kernel */ acl_max=506 fi if $ACL_MAX_ADJ is defined; then acl_max += $ACL_MAX_ADJ; fi Thanks, > > Thanks, > > > > > Thanks, > > > >> if [ $? -eq 0 ]; then > >> - echo 531 > >> + echo 507 > >> else > >> - echo 506 > >> + echo 532 > >> fi > >> ;; > >> *) > >> -- > >> 2.12.2.575.gb14f27f ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-04 17:39 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-28 13:13 [PATCH] attr: adbjust acl_max of f2fs Chao Yu 2017-05-03 7:29 ` Eryu Guan 2017-05-03 8:07 ` Chao Yu 2017-05-03 17:02 ` Jaegeuk Kim 2017-05-04 15:32 ` Chao Yu 2017-05-04 17:39 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).