All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
@ 2019-06-20  8:30 Yue Hu
  2019-06-20  8:32 ` Gao Xiang
  2019-06-21  7:01 ` Greg KH
  0 siblings, 2 replies; 9+ messages in thread
From: Yue Hu @ 2019-06-20  8:30 UTC (permalink / raw)


From: Yue Hu <huyue2@yulong.com>

erofs_xattr_security_handler is already marked __maybe_unused, no need
to add CONFIG_EROFS_FS_SECURITY condition.

Signed-off-by: Yue Hu <huyue2 at yulong.com>
---
 drivers/staging/erofs/xattr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index df40654..06024ac 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
 	.get	= erofs_xattr_generic_get,
 };
 
-#ifdef CONFIG_EROFS_FS_SECURITY
 const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
 	.prefix	= XATTR_SECURITY_PREFIX,
 	.flags	= EROFS_XATTR_INDEX_SECURITY,
 	.get	= erofs_xattr_generic_get,
 };
-#endif
 
 const struct xattr_handler *erofs_xattr_handlers[] = {
 	&erofs_xattr_user_handler,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  8:30 [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY Yue Hu
@ 2019-06-20  8:32 ` Gao Xiang
  2019-06-20  9:22   ` Chao Yu
  2019-06-20  9:25   ` Yue Hu
  2019-06-21  7:01 ` Greg KH
  1 sibling, 2 replies; 9+ messages in thread
From: Gao Xiang @ 2019-06-20  8:32 UTC (permalink / raw)


Hi Yue,

On 2019/6/20 16:30, Yue Hu wrote:
> From: Yue Hu <huyue2 at yulong.com>
> 
> erofs_xattr_security_handler is already marked __maybe_unused, no need
> to add CONFIG_EROFS_FS_SECURITY condition.
> 
> Signed-off-by: Yue Hu <huyue2 at yulong.com>
> ---
>  drivers/staging/erofs/xattr.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> index df40654..06024ac 100644
> --- a/drivers/staging/erofs/xattr.c
> +++ b/drivers/staging/erofs/xattr.c
> @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
>  	.get	= erofs_xattr_generic_get,
>  };
>  
> -#ifdef CONFIG_EROFS_FS_SECURITY
>  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
>  	.prefix	= XATTR_SECURITY_PREFIX,
>  	.flags	= EROFS_XATTR_INDEX_SECURITY,
>  	.get	= erofs_xattr_generic_get,
>  };
> -#endif

Thanks for your patch.

In that case...erofs_xattr_security_handler could be compiled into .rodata section?
I am not sure...

Thanks,
Gao Xiang

>  
>  const struct xattr_handler *erofs_xattr_handlers[] = {
>  	&erofs_xattr_user_handler,
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  8:32 ` Gao Xiang
@ 2019-06-20  9:22   ` Chao Yu
  2019-06-20  9:29     ` Yue Hu
  2019-06-20  9:25   ` Yue Hu
  1 sibling, 1 reply; 9+ messages in thread
From: Chao Yu @ 2019-06-20  9:22 UTC (permalink / raw)


On 2019/6/20 16:32, Gao Xiang wrote:
> Hi Yue,
> 
> On 2019/6/20 16:30, Yue Hu wrote:
>> From: Yue Hu <huyue2 at yulong.com>
>>
>> erofs_xattr_security_handler is already marked __maybe_unused, no need
>> to add CONFIG_EROFS_FS_SECURITY condition.

CONFIG_EROFS_FS_SECURITY is used as a control switch of erofs security labels
feature, but __maybe_unused is to avoid unneeded compiler warning on unused
variable, so I think we can't remove it.

Thanks,

>>
>> Signed-off-by: Yue Hu <huyue2 at yulong.com>
>> ---
>>  drivers/staging/erofs/xattr.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
>> index df40654..06024ac 100644
>> --- a/drivers/staging/erofs/xattr.c
>> +++ b/drivers/staging/erofs/xattr.c
>> @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
>>  	.get	= erofs_xattr_generic_get,
>>  };
>>  
>> -#ifdef CONFIG_EROFS_FS_SECURITY
>>  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
>>  	.prefix	= XATTR_SECURITY_PREFIX,
>>  	.flags	= EROFS_XATTR_INDEX_SECURITY,
>>  	.get	= erofs_xattr_generic_get,
>>  };
>> -#endif
> 
> Thanks for your patch.
> 
> In that case...erofs_xattr_security_handler could be compiled into .rodata section?
> I am not sure...
> 
> Thanks,
> Gao Xiang
> 
>>  
>>  const struct xattr_handler *erofs_xattr_handlers[] = {
>>  	&erofs_xattr_user_handler,
>>
> .
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  8:32 ` Gao Xiang
  2019-06-20  9:22   ` Chao Yu
@ 2019-06-20  9:25   ` Yue Hu
  2019-06-20  9:39     ` Gao Xiang
  1 sibling, 1 reply; 9+ messages in thread
From: Yue Hu @ 2019-06-20  9:25 UTC (permalink / raw)


On Thu, 20 Jun 2019 16:32:01 +0800
Gao Xiang <gaoxiang25@huawei.com> wrote:

> Hi Yue,
> 
> On 2019/6/20 16:30, Yue Hu wrote:
> > From: Yue Hu <huyue2 at yulong.com>
> > 
> > erofs_xattr_security_handler is already marked __maybe_unused, no need
> > to add CONFIG_EROFS_FS_SECURITY condition.
> > 
> > Signed-off-by: Yue Hu <huyue2 at yulong.com>
> > ---
> >  drivers/staging/erofs/xattr.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> > index df40654..06024ac 100644
> > --- a/drivers/staging/erofs/xattr.c
> > +++ b/drivers/staging/erofs/xattr.c
> > @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
> >  	.get	= erofs_xattr_generic_get,
> >  };
> >  
> > -#ifdef CONFIG_EROFS_FS_SECURITY
> >  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
> >  	.prefix	= XATTR_SECURITY_PREFIX,
> >  	.flags	= EROFS_XATTR_INDEX_SECURITY,
> >  	.get	= erofs_xattr_generic_get,
> >  };
> > -#endif  
> 
> Thanks for your patch.
> 
> In that case...erofs_xattr_security_handler could be compiled into .rodata section?
> I am not sure...

Yes, just like erofs_xattr_user_handler as below in System.map.

ffffffff820ec2a0 R erofs_xattr_security_handler
ffffffff820ec2e0 R erofs_xattr_trusted_handler
ffffffff820ec320 R erofs_xattr_user_handler

Thx.

> 
> Thanks,
> Gao Xiang
> 
> >  
> >  const struct xattr_handler *erofs_xattr_handlers[] = {
> >  	&erofs_xattr_user_handler,
> >   

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  9:22   ` Chao Yu
@ 2019-06-20  9:29     ` Yue Hu
  2019-06-20  9:52       ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Yue Hu @ 2019-06-20  9:29 UTC (permalink / raw)


On Thu, 20 Jun 2019 17:22:48 +0800
Chao Yu <yuchao0@huawei.com> wrote:

> On 2019/6/20 16:32, Gao Xiang wrote:
> > Hi Yue,
> > 
> > On 2019/6/20 16:30, Yue Hu wrote:  
> >> From: Yue Hu <huyue2 at yulong.com>
> >>
> >> erofs_xattr_security_handler is already marked __maybe_unused, no need
> >> to add CONFIG_EROFS_FS_SECURITY condition.  
> 
> CONFIG_EROFS_FS_SECURITY is used as a control switch of erofs security labels
> feature, but __maybe_unused is to avoid unneeded compiler warning on unused
> variable, so I think we can't remove it.

However, erofs_xattr_security_handler will not unused under CONFIG_EROFS_FS_SECURITY
condition, right?

Thx.

> 
> Thanks,
> 
> >>
> >> Signed-off-by: Yue Hu <huyue2 at yulong.com>
> >> ---
> >>  drivers/staging/erofs/xattr.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> >> index df40654..06024ac 100644
> >> --- a/drivers/staging/erofs/xattr.c
> >> +++ b/drivers/staging/erofs/xattr.c
> >> @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
> >>  	.get	= erofs_xattr_generic_get,
> >>  };
> >>  
> >> -#ifdef CONFIG_EROFS_FS_SECURITY
> >>  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
> >>  	.prefix	= XATTR_SECURITY_PREFIX,
> >>  	.flags	= EROFS_XATTR_INDEX_SECURITY,
> >>  	.get	= erofs_xattr_generic_get,
> >>  };
> >> -#endif  
> > 
> > Thanks for your patch.
> > 
> > In that case...erofs_xattr_security_handler could be compiled into .rodata section?
> > I am not sure...
> > 
> > Thanks,
> > Gao Xiang
> >   
> >>  
> >>  const struct xattr_handler *erofs_xattr_handlers[] = {
> >>  	&erofs_xattr_user_handler,
> >>  
> > .
> >   

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  9:25   ` Yue Hu
@ 2019-06-20  9:39     ` Gao Xiang
  0 siblings, 0 replies; 9+ messages in thread
From: Gao Xiang @ 2019-06-20  9:39 UTC (permalink / raw)




On 2019/6/20 17:25, Yue Hu wrote:
> On Thu, 20 Jun 2019 16:32:01 +0800
> Gao Xiang <gaoxiang25@huawei.com> wrote:
> 
>> Hi Yue,
>>
>> On 2019/6/20 16:30, Yue Hu wrote:
>>> From: Yue Hu <huyue2 at yulong.com>
>>>
>>> erofs_xattr_security_handler is already marked __maybe_unused, no need
>>> to add CONFIG_EROFS_FS_SECURITY condition.
>>>
>>> Signed-off-by: Yue Hu <huyue2 at yulong.com>
>>> ---
>>>  drivers/staging/erofs/xattr.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
>>> index df40654..06024ac 100644
>>> --- a/drivers/staging/erofs/xattr.c
>>> +++ b/drivers/staging/erofs/xattr.c
>>> @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
>>>  	.get	= erofs_xattr_generic_get,
>>>  };
>>>  
>>> -#ifdef CONFIG_EROFS_FS_SECURITY
>>>  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
>>>  	.prefix	= XATTR_SECURITY_PREFIX,
>>>  	.flags	= EROFS_XATTR_INDEX_SECURITY,
>>>  	.get	= erofs_xattr_generic_get,
>>>  };
>>> -#endif  
>>
>> Thanks for your patch.
>>
>> In that case...erofs_xattr_security_handler could be compiled into .rodata section?
>> I am not sure...
> 
> Yes, just like erofs_xattr_user_handler as below in System.map.
> 
> ffffffff820ec2a0 R erofs_xattr_security_handler
> ffffffff820ec2e0 R erofs_xattr_trusted_handler
> ffffffff820ec320 R erofs_xattr_user_handler

As a usual practice, CONFIG_{EXT2,EXT4,F2FS,EROFS}_FS_SECURITY are defined as
kernel configuations.

It seems that for ext2/ext4 they leave

const struct xattr_handler ext2_xattr_security_handler = {

in xattr_security.c and the Makefiles are similar as

fs/ext2/Makefile
13:ext2-$(CONFIG_EXT2_FS_SECURITY)       += xattr_security.o

But for f2fs, f2fs_xattr_security_handler is not wrapped with any configuration.

Actually I think that is not a big deal, I'd like to listen Chao and Greg's
idea about this...

Thanks,
Gao Xiang

> 
> Thx.
> 
>>
>> Thanks,
>> Gao Xiang
>>
>>>  
>>>  const struct xattr_handler *erofs_xattr_handlers[] = {
>>>  	&erofs_xattr_user_handler,
>>>   
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  9:29     ` Yue Hu
@ 2019-06-20  9:52       ` Chao Yu
  2019-06-20  9:56         ` Yue Hu
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2019-06-20  9:52 UTC (permalink / raw)


On 2019/6/20 17:29, Yue Hu wrote:
> On Thu, 20 Jun 2019 17:22:48 +0800
> Chao Yu <yuchao0@huawei.com> wrote:
> 
>> On 2019/6/20 16:32, Gao Xiang wrote:
>>> Hi Yue,
>>>
>>> On 2019/6/20 16:30, Yue Hu wrote:  
>>>> From: Yue Hu <huyue2 at yulong.com>
>>>>
>>>> erofs_xattr_security_handler is already marked __maybe_unused, no need
>>>> to add CONFIG_EROFS_FS_SECURITY condition.  
>>
>> CONFIG_EROFS_FS_SECURITY is used as a control switch of erofs security labels
>> feature, but __maybe_unused is to avoid unneeded compiler warning on unused
>> variable, so I think we can't remove it.
> 
> However, erofs_xattr_security_handler will not unused under CONFIG_EROFS_FS_SECURITY
> condition, right?

Yes, we will referred it in erofs_xattr_handlers anyway, so, maybe we can remove
__maybe_unused instead?

Thanks,

> 
> Thx.
> 
>>
>> Thanks,
>>
>>>>
>>>> Signed-off-by: Yue Hu <huyue2 at yulong.com>
>>>> ---
>>>>  drivers/staging/erofs/xattr.c | 2 --
>>>>  1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
>>>> index df40654..06024ac 100644
>>>> --- a/drivers/staging/erofs/xattr.c
>>>> +++ b/drivers/staging/erofs/xattr.c
>>>> @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
>>>>  	.get	= erofs_xattr_generic_get,
>>>>  };
>>>>  
>>>> -#ifdef CONFIG_EROFS_FS_SECURITY
>>>>  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
>>>>  	.prefix	= XATTR_SECURITY_PREFIX,
>>>>  	.flags	= EROFS_XATTR_INDEX_SECURITY,
>>>>  	.get	= erofs_xattr_generic_get,
>>>>  };
>>>> -#endif  
>>>
>>> Thanks for your patch.
>>>
>>> In that case...erofs_xattr_security_handler could be compiled into .rodata section?
>>> I am not sure...
>>>
>>> Thanks,
>>> Gao Xiang
>>>   
>>>>  
>>>>  const struct xattr_handler *erofs_xattr_handlers[] = {
>>>>  	&erofs_xattr_user_handler,
>>>>  
>>> .
>>>   
> 
> .
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  9:52       ` Chao Yu
@ 2019-06-20  9:56         ` Yue Hu
  0 siblings, 0 replies; 9+ messages in thread
From: Yue Hu @ 2019-06-20  9:56 UTC (permalink / raw)


On Thu, 20 Jun 2019 17:52:39 +0800
Chao Yu <yuchao0@huawei.com> wrote:

> On 2019/6/20 17:29, Yue Hu wrote:
> > On Thu, 20 Jun 2019 17:22:48 +0800
> > Chao Yu <yuchao0@huawei.com> wrote:
> >   
> >> On 2019/6/20 16:32, Gao Xiang wrote:  
> >>> Hi Yue,
> >>>
> >>> On 2019/6/20 16:30, Yue Hu wrote:    
> >>>> From: Yue Hu <huyue2 at yulong.com>
> >>>>
> >>>> erofs_xattr_security_handler is already marked __maybe_unused, no need
> >>>> to add CONFIG_EROFS_FS_SECURITY condition.    
> >>
> >> CONFIG_EROFS_FS_SECURITY is used as a control switch of erofs security labels
> >> feature, but __maybe_unused is to avoid unneeded compiler warning on unused
> >> variable, so I think we can't remove it.  
> > 
> > However, erofs_xattr_security_handler will not unused under CONFIG_EROFS_FS_SECURITY
> > condition, right?  
> 
> Yes, we will referred it in erofs_xattr_handlers anyway, so, maybe we can remove
> __maybe_unused instead?

It's good to me.

Thx.

> 
> Thanks,
> 
> > 
> > Thx.
> >   
> >>
> >> Thanks,
> >>  
> >>>>
> >>>> Signed-off-by: Yue Hu <huyue2 at yulong.com>
> >>>> ---
> >>>>  drivers/staging/erofs/xattr.c | 2 --
> >>>>  1 file changed, 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> >>>> index df40654..06024ac 100644
> >>>> --- a/drivers/staging/erofs/xattr.c
> >>>> +++ b/drivers/staging/erofs/xattr.c
> >>>> @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
> >>>>  	.get	= erofs_xattr_generic_get,
> >>>>  };
> >>>>  
> >>>> -#ifdef CONFIG_EROFS_FS_SECURITY
> >>>>  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
> >>>>  	.prefix	= XATTR_SECURITY_PREFIX,
> >>>>  	.flags	= EROFS_XATTR_INDEX_SECURITY,
> >>>>  	.get	= erofs_xattr_generic_get,
> >>>>  };
> >>>> -#endif    
> >>>
> >>> Thanks for your patch.
> >>>
> >>> In that case...erofs_xattr_security_handler could be compiled into .rodata section?
> >>> I am not sure...
> >>>
> >>> Thanks,
> >>> Gao Xiang
> >>>     
> >>>>  
> >>>>  const struct xattr_handler *erofs_xattr_handlers[] = {
> >>>>  	&erofs_xattr_user_handler,
> >>>>    
> >>> .
> >>>     
> > 
> > .
> >   

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY
  2019-06-20  8:30 [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY Yue Hu
  2019-06-20  8:32 ` Gao Xiang
@ 2019-06-21  7:01 ` Greg KH
  1 sibling, 0 replies; 9+ messages in thread
From: Greg KH @ 2019-06-21  7:01 UTC (permalink / raw)


On Thu, Jun 20, 2019@04:30:04PM +0800, Yue Hu wrote:
> From: Yue Hu <huyue2 at yulong.com>
> 
> erofs_xattr_security_handler is already marked __maybe_unused, no need
> to add CONFIG_EROFS_FS_SECURITY condition.
> 
> Signed-off-by: Yue Hu <huyue2 at yulong.com>
> ---
>  drivers/staging/erofs/xattr.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
> index df40654..06024ac 100644
> --- a/drivers/staging/erofs/xattr.c
> +++ b/drivers/staging/erofs/xattr.c
> @@ -499,13 +499,11 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
>  	.get	= erofs_xattr_generic_get,
>  };
>  
> -#ifdef CONFIG_EROFS_FS_SECURITY
>  const struct xattr_handler __maybe_unused erofs_xattr_security_handler = {
>  	.prefix	= XATTR_SECURITY_PREFIX,
>  	.flags	= EROFS_XATTR_INDEX_SECURITY,
>  	.get	= erofs_xattr_generic_get,
>  };
> -#endif

It's nicer just to leave this as-is for now, the memory savings isn't
much at all.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-06-21  7:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-20  8:30 [PATCH] staging: erofs: remove needless CONFIG_EROFS_FS_SECURITY Yue Hu
2019-06-20  8:32 ` Gao Xiang
2019-06-20  9:22   ` Chao Yu
2019-06-20  9:29     ` Yue Hu
2019-06-20  9:52       ` Chao Yu
2019-06-20  9:56         ` Yue Hu
2019-06-20  9:25   ` Yue Hu
2019-06-20  9:39     ` Gao Xiang
2019-06-21  7:01 ` Greg KH

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.