[PATCH] device-dax: include bus.h in super.c

[PATCH] device-dax: include bus.h in super.c

[Jason Yan]

This addresses the following sparse warning:

drivers/dax/super.c:452:6: warning: symbol 'run_dax' was not declared.
Should it be static?

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 drivers/dax/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index edc279be3e59..2cf6faf265c5 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -16,6 +16,7 @@
 #include <linux/dax.h>
 #include <linux/fs.h>
 #include "dax-private.h"
+#include "bus.h"
 
 static dev_t dax_devt;
 DEFINE_STATIC_SRCU(dax_srcu);
-- 
2.25.4
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH] device-dax: include bus.h in super.c

[Dan Williams]

On Fri, Sep 25, 2020 at 2:17 AM Jason Yan <yanaijie@huawei.com> wrote:
>
> This addresses the following sparse warning:
>
> drivers/dax/super.c:452:6: warning: symbol 'run_dax' was not declared.
> Should it be static?

run_dax() is a core helper defined in drivers/dax/super.c that is
meant to hide the definition of 'struct dax_device' from the wider
kernel that does not need to poke into its internals. There's also no
need for drivers/dax/super.c to be given knowledge of other core
details that are contained within bus.h. So, I think this patch
provides no value and goes against the principle of least privilege
(https://en.wikipedia.org/wiki/Principle_of_least_privilege)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH] device-dax: include bus.h in super.c

[Jason Yan]

Hi Dan，

在 2020/9/26 2:24, Dan Williams 写道:
> On Fri, Sep 25, 2020 at 2:17 AM Jason Yan <yanaijie@huawei.com> wrote:
>>
>> This addresses the following sparse warning:
>>
>> drivers/dax/super.c:452:6: warning: symbol 'run_dax' was not declared.
>> Should it be static?
> 
> run_dax() is a core helper defined in drivers/dax/super.c that is
> meant to hide the definition of 'struct dax_device' from the wider
> kernel that does not need to poke into its internals. There's also no
> need for drivers/dax/super.c to be given knowledge of other core
> details that are contained within bus.h. So, I think this patch
> provides no value and goes against the principle of least privilege
> (https://en.wikipedia.org/wiki/Principle_of_least_privilege)
> 

Sorry I did not get what you mean. I only included the internal bus.h
which is drivers/dax/bus.h. Why is this affecting the other part of the
kernel?

Thanks,
Jason
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH] device-dax: include bus.h in super.c

[Jason Yan]

在 2020/9/28 9:15, Jason Yan 写道:
> Hi Dan，
> 
> 在 2020/9/26 2:24, Dan Williams 写道:
>> On Fri, Sep 25, 2020 at 2:17 AM Jason Yan <yanaijie@huawei.com> wrote:
>>>
>>> This addresses the following sparse warning:
>>>
>>> drivers/dax/super.c:452:6: warning: symbol 'run_dax' was not declared.
>>> Should it be static?
>>
>> run_dax() is a core helper defined in drivers/dax/super.c that is
>> meant to hide the definition of 'struct dax_device' from the wider
>> kernel that does not need to poke into its internals. There's also no
>> need for drivers/dax/super.c to be given knowledge of other core
>> details that are contained within bus.h. So, I think this patch
>> provides no value and goes against the principle of least privilege
>> (https://en.wikipedia.org/wiki/Principle_of_least_privilege)
>>
> 
> Sorry I did not get what you mean. I only included the internal bus.h
> which is drivers/dax/bus.h. Why is this affecting the other part of the
> kernel?
> 

And I did that because of run_dax() is declared in the drivers/dax/bus.h 
and defined in drivers/dax/super.c. The is totally an internal change in 
dax and can fix the sparse warning.

> Thanks,
> Jason
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH] device-dax: include bus.h in super.c

[Dan Williams]

On Sun, Sep 27, 2020 at 6:15 PM Jason Yan <yanaijie@huawei.com> wrote:
>
> Hi Dan，
>
> 在 2020/9/26 2:24, Dan Williams 写道:
> > On Fri, Sep 25, 2020 at 2:17 AM Jason Yan <yanaijie@huawei.com> wrote:
> >>
> >> This addresses the following sparse warning:
> >>
> >> drivers/dax/super.c:452:6: warning: symbol 'run_dax' was not declared.
> >> Should it be static?
> >
> > run_dax() is a core helper defined in drivers/dax/super.c that is
> > meant to hide the definition of 'struct dax_device' from the wider
> > kernel that does not need to poke into its internals. There's also no
> > need for drivers/dax/super.c to be given knowledge of other core
> > details that are contained within bus.h. So, I think this patch
> > provides no value and goes against the principle of least privilege
> > (https://en.wikipedia.org/wiki/Principle_of_least_privilege)
> >
>
> Sorry I did not get what you mean. I only included the internal bus.h
> which is drivers/dax/bus.h. Why is this affecting the other part of the
> kernel?

It's not affecting other parts of the kernel. driver/dax/super.c does
not need the other definitions in bus.h. You could make the argument
that the definition of run_dax() needs to move somewhere else that
super.c could include, but just blindly giving super.c access to all
the other definitions in bus.h is the wrong fix in my view.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org