| Message ID | 20200925091806.1860663-1-yanaijie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | device-dax: include bus.h in super.c | expand |
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)
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
在 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
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.
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);
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(+)