Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <201903140234.4FpTWdW3%lkp@intel.com> <20190314083758.GA16658@quack2.suse.cz>
In-Reply-To: <20190314083758.GA16658@quack2.suse.cz>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Thu, 14 Mar 2019 14:01:18 +0200
Message-ID: <CAOQ4uxhZH9=U63J1_bVrMbNO-Quy-8S300Qi6VmZxvKwYCogQQ@mail.gmail.com>
Subject: Re: fs/notify/fanotify/fanotify.c:198:2: note: in expansion of macro 'pr_warn_ratelimited'
To:     Jan Kara <jack@suse.cz>
Cc:     kbuild-all@01.org, linux-kernel <linux-kernel@vger.kernel.org>,
        linux-mips@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
        Paul Burton <paul.burton@mips.com>,
        James Hogan <jhogan@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 14, 2019 at 10:37 AM Jan Kara <jack@suse.cz> wrote:
>
> AFAICS this is the known problem with weird mips definitions of
> __kernel_fsid_t which uses long whereas all other architectures use int,
> right? Seeing that mips can actually have 8-byte longs, I guess this
> bogosity is just wired in the kernel API and we cannot easily fix it in
> mips (mips guys, correct me if I'm wrong). So what if we just
> unconditionally typed printed values to unsigned int to silence the
> warning?

I don't understand why. To me that sounds like papering over a bug.

See this reply from mips developer Paul Burton:
https://marc.info/?l=linux-fsdevel&m=154783680019904&w=2
mips developers have not replied to the question why __kernel_fsid_t
should use long.

My concern is that we expose __kernel_fsid_t type in uapi header
in struct fanotify_event_info_fid. We should make sure this type
is consistent with glibc's fsid_t.

For reference, the statfs(2) man page says that on Linux
"...fsid_t is defined as struct { int val[2]; }".

Besides, it looks like __kernel_fsid_t got let behind on the
mips posix_types cleanup:
bb8ac181a5cf bury __kernel_nlink_t, make internal nlink_t consistent
86fcd10e9a57 mips: Use generic posix_types.h

To me this seems like an issue that mips developers should
advise on the solution.

Thanks,
Amir.

>
>                                                                 Honza
>
> On Thu 14-03-19 02:31:52, kbuild test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   5453a3df2a5eb49bc24615d4cf0d66b2aae05e5f
> > commit: e9e0c8903009477b630e37a8b6364b26a00720da fanotify: encode file identifier for FAN_REPORT_FID
> > date:   5 weeks ago
> > config: mips-allmodconfig (attached as .config)
> > compiler: mips-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         git checkout e9e0c8903009477b630e37a8b6364b26a00720da
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=7.2.0 make.cross ARCH=mips
> >
> > All warnings (new ones prefixed by >>):
> >
> >    In file included from include/linux/kernel.h:14:0,
> >                     from include/linux/list.h:9,
> >                     from include/linux/preempt.h:11,
> >                     from include/linux/spinlock.h:51,
> >                     from include/linux/fdtable.h:11,
> >                     from fs/notify/fanotify/fanotify.c:3:
> >    fs/notify/fanotify/fanotify.c: In function 'fanotify_encode_fid':
> >    include/linux/kern_levels.h:5:18: warning: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'long int' [-Wformat=]
> >     #define KERN_SOH "\001"  /* ASCII Start Of Header */
> >                      ^
> >    include/linux/printk.h:424:10: note: in definition of macro 'printk_ratelimited'
> >       printk(fmt, ##__VA_ARGS__);    \
> >              ^~~
> >    include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
> >     #define KERN_WARNING KERN_SOH "4" /* warning conditions */
> >                          ^~~~~~~~
> >    include/linux/printk.h:440:21: note: in expansion of macro 'KERN_WARNING'
> >      printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> >                         ^~~~~~~~~~~~
> > >> fs/notify/fanotify/fanotify.c:198:2: note: in expansion of macro 'pr_warn_ratelimited'
> >      pr_warn_ratelimited("fanotify: failed to encode fid (fsid=%x.%x, "
> >      ^~~~~~~~~~~~~~~~~~~
> >    fs/notify/fanotify/fanotify.c:198:61: note: format string is defined here
> >      pr_warn_ratelimited("fanotify: failed to encode fid (fsid=%x.%x, "
> >                                                                ~^
> >                                                                %lx
> >    In file included from include/linux/kernel.h:14:0,
> >                     from include/linux/list.h:9,
> >                     from include/linux/preempt.h:11,
> >                     from include/linux/spinlock.h:51,
> >                     from include/linux/fdtable.h:11,
> >                     from fs/notify/fanotify/fanotify.c:3:
> >    include/linux/kern_levels.h:5:18: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'long int' [-Wformat=]
> >     #define KERN_SOH "\001"  /* ASCII Start Of Header */
> >                      ^
> >    include/linux/printk.h:424:10: note: in definition of macro 'printk_ratelimited'
> >       printk(fmt, ##__VA_ARGS__);    \
> >              ^~~
> >    include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
> >     #define KERN_WARNING KERN_SOH "4" /* warning conditions */
> >                          ^~~~~~~~
> >    include/linux/printk.h:440:21: note: in expansion of macro 'KERN_WARNING'
> >      printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
> >                         ^~~~~~~~~~~~
> > >> fs/notify/fanotify/fanotify.c:198:2: note: in expansion of macro 'pr_warn_ratelimited'
> >      pr_warn_ratelimited("fanotify: failed to encode fid (fsid=%x.%x, "
> >      ^~~~~~~~~~~~~~~~~~~
> >    fs/notify/fanotify/fanotify.c:198:64: note: format string is defined here
> >      pr_warn_ratelimited("fanotify: failed to encode fid (fsid=%x.%x, "
> >                                                                   ~^
> >                                                                   %lx
> >
> > vim +/pr_warn_ratelimited +198 fs/notify/fanotify/fanotify.c
> >
> >    154
> >    155        static int fanotify_encode_fid(struct fanotify_event *event,
> >    156                                       const struct path *path, gfp_t gfp)
> >    157        {
> >    158                struct fanotify_fid *fid = &event->fid;
> >    159                int dwords, bytes = 0;
> >    160                struct kstatfs stat;
> >    161                int err, type;
> >    162
> >    163                stat.f_fsid.val[0] = stat.f_fsid.val[1] = 0;
> >    164                fid->ext_fh = NULL;
> >    165                dwords = 0;
> >    166                err = -ENOENT;
> >    167                type = exportfs_encode_inode_fh(d_inode(path->dentry), NULL, &dwords,
> >    168                                                NULL);
> >    169                if (!dwords)
> >    170                        goto out_err;
> >    171
> >    172                err = vfs_statfs(path, &stat);
> >    173                if (err)
> >    174                        goto out_err;
> >    175
> >    176                bytes = dwords << 2;
> >    177                if (bytes > FANOTIFY_INLINE_FH_LEN) {
> >    178                        /* Treat failure to allocate fh as failure to allocate event */
> >    179                        err = -ENOMEM;
> >    180                        fid->ext_fh = kmalloc(bytes, gfp);
> >    181                        if (!fid->ext_fh)
> >    182                                goto out_err;
> >    183                }
> >    184
> >    185                type = exportfs_encode_inode_fh(d_inode(path->dentry),
> >    186                                                fanotify_fid_fh(fid, bytes), &dwords,
> >    187                                                NULL);
> >    188                err = -EINVAL;
> >    189                if (!type || type == FILEID_INVALID || bytes != dwords << 2)
> >    190                        goto out_err;
> >    191
> >    192                fid->fsid = stat.f_fsid;
> >    193                event->fh_len = bytes;
> >    194
> >    195                return type;
> >    196
> >    197        out_err:
> >  > 198                pr_warn_ratelimited("fanotify: failed to encode fid (fsid=%x.%x, "
> >    199                                    "type=%d, bytes=%d, err=%i)\n",
> >    200                                    stat.f_fsid.val[0], stat.f_fsid.val[1],
> >    201                                    type, bytes, err);
> >    202                kfree(fid->ext_fh);
> >    203                fid->ext_fh = NULL;
> >    204                event->fh_len = 0;
> >    205
> >    206                return FILEID_INVALID;
> >    207        }
> >    208
> >
> > ---
> > 0-DAY kernel test infrastructure                Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
>
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR