expfs.c has a bunch of dprintk statements which are unusable due to:
#define dprintk(fmt, args...) do{}while(0)
Use pr_debug so that they can be enabled dynamically.
Also make some minor changes to the debug statements to fix some
incorrect types, and remove __func__ which can be handled by dynamic
debug separately.
Signed-off-by: David Disseldorp <[email protected]>
---
fs/exportfs/expfs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index c648a493faf23..3204bd33e4e8a 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -18,7 +18,7 @@
#include <linux/sched.h>
#include <linux/cred.h>
-#define dprintk(fmt, args...) do{}while(0)
+#define dprintk(fmt, args...) pr_debug(fmt, ##args)
static int get_name(const struct path *path, char *name, struct dentry *child);
@@ -132,8 +132,8 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
inode_unlock(dentry->d_inode);
if (IS_ERR(parent)) {
- dprintk("%s: get_parent of %ld failed, err %d\n",
- __func__, dentry->d_inode->i_ino, PTR_ERR(parent));
+ dprintk("get_parent of %lu failed, err %ld\n",
+ dentry->d_inode->i_ino, PTR_ERR(parent));
return parent;
}
@@ -147,7 +147,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
dprintk("%s: found name: %s\n", __func__, nbuf);
tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
if (IS_ERR(tmp)) {
- dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
+ dprintk("lookup failed: %ld\n", PTR_ERR(tmp));
err = PTR_ERR(tmp);
goto out_err;
}
--
2.35.3
On Fri, 21 Oct 2022, David Disseldorp wrote:
> expfs.c has a bunch of dprintk statements which are unusable due to:
> #define dprintk(fmt, args...) do{}while(0)
> Use pr_debug so that they can be enabled dynamically.
> Also make some minor changes to the debug statements to fix some
> incorrect types, and remove __func__ which can be handled by dynamic
> debug separately.
>
> Signed-off-by: David Disseldorp <[email protected]>
Reviewed-by: NeilBrown <[email protected]>
Thanks,
NeilBrown
> ---
> fs/exportfs/expfs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index c648a493faf23..3204bd33e4e8a 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -18,7 +18,7 @@
> #include <linux/sched.h>
> #include <linux/cred.h>
>
> -#define dprintk(fmt, args...) do{}while(0)
> +#define dprintk(fmt, args...) pr_debug(fmt, ##args)
>
>
> static int get_name(const struct path *path, char *name, struct dentry *child);
> @@ -132,8 +132,8 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
> inode_unlock(dentry->d_inode);
>
> if (IS_ERR(parent)) {
> - dprintk("%s: get_parent of %ld failed, err %d\n",
> - __func__, dentry->d_inode->i_ino, PTR_ERR(parent));
> + dprintk("get_parent of %lu failed, err %ld\n",
> + dentry->d_inode->i_ino, PTR_ERR(parent));
> return parent;
> }
>
> @@ -147,7 +147,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
> dprintk("%s: found name: %s\n", __func__, nbuf);
> tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
> if (IS_ERR(tmp)) {
> - dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
> + dprintk("lookup failed: %ld\n", PTR_ERR(tmp));
> err = PTR_ERR(tmp);
> goto out_err;
> }
> --
> 2.35.3
>
>
> On Oct 23, 2022, at 6:11 PM, NeilBrown <[email protected]> wrote:
>
> On Fri, 21 Oct 2022, David Disseldorp wrote:
>> expfs.c has a bunch of dprintk statements which are unusable due to:
>> #define dprintk(fmt, args...) do{}while(0)
>> Use pr_debug so that they can be enabled dynamically.
>> Also make some minor changes to the debug statements to fix some
>> incorrect types, and remove __func__ which can be handled by dynamic
>> debug separately.
>>
>> Signed-off-by: David Disseldorp <[email protected]>
>
> Reviewed-by: NeilBrown <[email protected]>
>
> Thanks,
> NeilBrown
I don't think we're the maintainers of expfs.c ?
$ scripts/get_maintainer.pl fs/exportfs/expfs.c
Christian Brauner <[email protected]> (commit_signer:2/2=100%,authored:1/2=50%,added_lines:3/6=50%,removed_lines:2/6=33%)
Al Viro <[email protected]> (commit_signer:1/2=50%,authored:1/2=50%,added_lines:3/6=50%,removed_lines:4/6=67%)
Miklos Szeredi <[email protected]> (commit_signer:1/2=50%)
Amir Goldstein <[email protected]> (commit_signer:1/2=50%)
[email protected] (open list)
But maybe MAINTAINERS needs to be fixed. There's no entry
there for fs/exportfs.
>> ---
>> fs/exportfs/expfs.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
>> index c648a493faf23..3204bd33e4e8a 100644
>> --- a/fs/exportfs/expfs.c
>> +++ b/fs/exportfs/expfs.c
>> @@ -18,7 +18,7 @@
>> #include <linux/sched.h>
>> #include <linux/cred.h>
>>
>> -#define dprintk(fmt, args...) do{}while(0)
>> +#define dprintk(fmt, args...) pr_debug(fmt, ##args)
>>
>>
>> static int get_name(const struct path *path, char *name, struct dentry *child);
>> @@ -132,8 +132,8 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
>> inode_unlock(dentry->d_inode);
>>
>> if (IS_ERR(parent)) {
>> - dprintk("%s: get_parent of %ld failed, err %d\n",
>> - __func__, dentry->d_inode->i_ino, PTR_ERR(parent));
>> + dprintk("get_parent of %lu failed, err %ld\n",
>> + dentry->d_inode->i_ino, PTR_ERR(parent));
>> return parent;
>> }
>>
>> @@ -147,7 +147,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
>> dprintk("%s: found name: %s\n", __func__, nbuf);
>> tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
>> if (IS_ERR(tmp)) {
>> - dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
>> + dprintk("lookup failed: %ld\n", PTR_ERR(tmp));
>> err = PTR_ERR(tmp);
>> goto out_err;
>> }
>> --
>> 2.35.3
>>
>>
--
Chuck Lever
On Tue, 25 Oct 2022, Chuck Lever III wrote:
>
> > On Oct 23, 2022, at 6:11 PM, NeilBrown <[email protected]> wrote:
> >
> > On Fri, 21 Oct 2022, David Disseldorp wrote:
> >> expfs.c has a bunch of dprintk statements which are unusable due to:
> >> #define dprintk(fmt, args...) do{}while(0)
> >> Use pr_debug so that they can be enabled dynamically.
> >> Also make some minor changes to the debug statements to fix some
> >> incorrect types, and remove __func__ which can be handled by dynamic
> >> debug separately.
> >>
> >> Signed-off-by: David Disseldorp <[email protected]>
> >
> > Reviewed-by: NeilBrown <[email protected]>
> >
> > Thanks,
> > NeilBrown
>
> I don't think we're the maintainers of expfs.c ?
>
> $ scripts/get_maintainer.pl fs/exportfs/expfs.c
> Christian Brauner <[email protected]> (commit_signer:2/2=100%,authored:1/2=50%,added_lines:3/6=50%,removed_lines:2/6=33%)
> Al Viro <[email protected]> (commit_signer:1/2=50%,authored:1/2=50%,added_lines:3/6=50%,removed_lines:4/6=67%)
> Miklos Szeredi <[email protected]> (commit_signer:1/2=50%)
> Amir Goldstein <[email protected]> (commit_signer:1/2=50%)
> [email protected] (open list)
>
> But maybe MAINTAINERS needs to be fixed. There's no entry
> there for fs/exportfs.
Looking at recent commits, patches come in through multiple different
trees.
nfsd certainly has an interest in expfs.c. The only other user is
name_to_handle/open_by_handle API.
I see it as primarily nfsd functionality which is useful enough to be
exported directly to user-space.
(It was created by me when I was nfsd maintainer - does that count?)
So I would support the suggestion of updating MAINTAINERS to include
fs/exportfs/ in the NFSD section.
Having said that - given your apparent preference of tracepoints for
tracing in nfsd - I suspect you might ask for a somewhat different patch
:-)
Thanks,
NeilBrown
>
>
> >> ---
> >> fs/exportfs/expfs.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> >> index c648a493faf23..3204bd33e4e8a 100644
> >> --- a/fs/exportfs/expfs.c
> >> +++ b/fs/exportfs/expfs.c
> >> @@ -18,7 +18,7 @@
> >> #include <linux/sched.h>
> >> #include <linux/cred.h>
> >>
> >> -#define dprintk(fmt, args...) do{}while(0)
> >> +#define dprintk(fmt, args...) pr_debug(fmt, ##args)
> >>
> >>
> >> static int get_name(const struct path *path, char *name, struct dentry *child);
> >> @@ -132,8 +132,8 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
> >> inode_unlock(dentry->d_inode);
> >>
> >> if (IS_ERR(parent)) {
> >> - dprintk("%s: get_parent of %ld failed, err %d\n",
> >> - __func__, dentry->d_inode->i_ino, PTR_ERR(parent));
> >> + dprintk("get_parent of %lu failed, err %ld\n",
> >> + dentry->d_inode->i_ino, PTR_ERR(parent));
> >> return parent;
> >> }
> >>
> >> @@ -147,7 +147,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
> >> dprintk("%s: found name: %s\n", __func__, nbuf);
> >> tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
> >> if (IS_ERR(tmp)) {
> >> - dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
> >> + dprintk("lookup failed: %ld\n", PTR_ERR(tmp));
> >> err = PTR_ERR(tmp);
> >> goto out_err;
> >> }
> >> --
> >> 2.35.3
> >>
> >>
>
> --
> Chuck Lever
>
>
>
>
> On Oct 24, 2022, at 6:09 PM, NeilBrown <[email protected]> wrote:
>
> On Tue, 25 Oct 2022, Chuck Lever III wrote:
>>
>>> On Oct 23, 2022, at 6:11 PM, NeilBrown <[email protected]> wrote:
>>>
>>> On Fri, 21 Oct 2022, David Disseldorp wrote:
>>>> expfs.c has a bunch of dprintk statements which are unusable due to:
>>>> #define dprintk(fmt, args...) do{}while(0)
>>>> Use pr_debug so that they can be enabled dynamically.
>>>> Also make some minor changes to the debug statements to fix some
>>>> incorrect types, and remove __func__ which can be handled by dynamic
>>>> debug separately.
>>>>
>>>> Signed-off-by: David Disseldorp <[email protected]>
>>>
>>> Reviewed-by: NeilBrown <[email protected]>
>>>
>>> Thanks,
>>> NeilBrown
>>
>> I don't think we're the maintainers of expfs.c ?
>>
>> $ scripts/get_maintainer.pl fs/exportfs/expfs.c
>> Christian Brauner <[email protected]> (commit_signer:2/2=100%,authored:1/2=50%,added_lines:3/6=50%,removed_lines:2/6=33%)
>> Al Viro <[email protected]> (commit_signer:1/2=50%,authored:1/2=50%,added_lines:3/6=50%,removed_lines:4/6=67%)
>> Miklos Szeredi <[email protected]> (commit_signer:1/2=50%)
>> Amir Goldstein <[email protected]> (commit_signer:1/2=50%)
>> [email protected] (open list)
>>
>> But maybe MAINTAINERS needs to be fixed. There's no entry
>> there for fs/exportfs.
>
> Looking at recent commits, patches come in through multiple different
> trees.
> nfsd certainly has an interest in expfs.c. The only other user is
> name_to_handle/open_by_handle API.
> I see it as primarily nfsd functionality which is useful enough to be
> exported directly to user-space.
> (It was created by me when I was nfsd maintainer - does that count?)
I can mechanically take the patch through nfsd if no-one objects.
My concern now is that it gets proper review. It's not an area
I'm especially familiar with.
> So I would support the suggestion of updating MAINTAINERS to include
> fs/exportfs/ in the NFSD section.
No problem with that, if my co-maintainer agrees.
> Having said that - given your apparent preference of tracepoints for
> tracing in nfsd - I suspect you might ask for a somewhat different patch
> :-)
I've certainly had that thought, but I don't think there currently is
a trace subsystem defined for exportfs.
> Thanks,
> NeilBrown
>
>
>>
>>
>>>> ---
>>>> fs/exportfs/expfs.c | 8 ++++----
>>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
>>>> index c648a493faf23..3204bd33e4e8a 100644
>>>> --- a/fs/exportfs/expfs.c
>>>> +++ b/fs/exportfs/expfs.c
>>>> @@ -18,7 +18,7 @@
>>>> #include <linux/sched.h>
>>>> #include <linux/cred.h>
>>>>
>>>> -#define dprintk(fmt, args...) do{}while(0)
>>>> +#define dprintk(fmt, args...) pr_debug(fmt, ##args)
>>>>
>>>>
>>>> static int get_name(const struct path *path, char *name, struct dentry *child);
>>>> @@ -132,8 +132,8 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
>>>> inode_unlock(dentry->d_inode);
>>>>
>>>> if (IS_ERR(parent)) {
>>>> - dprintk("%s: get_parent of %ld failed, err %d\n",
>>>> - __func__, dentry->d_inode->i_ino, PTR_ERR(parent));
>>>> + dprintk("get_parent of %lu failed, err %ld\n",
>>>> + dentry->d_inode->i_ino, PTR_ERR(parent));
>>>> return parent;
>>>> }
>>>>
>>>> @@ -147,7 +147,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
>>>> dprintk("%s: found name: %s\n", __func__, nbuf);
>>>> tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
>>>> if (IS_ERR(tmp)) {
>>>> - dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
>>>> + dprintk("lookup failed: %ld\n", PTR_ERR(tmp));
>>>> err = PTR_ERR(tmp);
>>>> goto out_err;
>>>> }
>>>> --
>>>> 2.35.3
>>>>
>>>>
>>
>> --
>> Chuck Lever
--
Chuck Lever