diff mbox series

device-dax: include bus.h in super.c

Message ID 20200925091806.1860663-1-yanaijie@huawei.com (mailing list archive)
State New
Headers show
Series device-dax: include bus.h in super.c | expand

Commit Message

Jason Yan Sept. 25, 2020, 9:18 a.m. UTC
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(+)

Comments

Dan Williams Sept. 25, 2020, 6:24 p.m. UTC | #1
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)
Jason Yan Sept. 28, 2020, 1:15 a.m. UTC | #2
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
Jason Yan Sept. 28, 2020, 1:18 a.m. UTC | #3
在 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
Dan Williams Sept. 28, 2020, 7:16 p.m. UTC | #4
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 mbox series

Patch

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);