public inbox for linux-next@vger.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Baokun Li <libaokun1@huawei.com>
Cc: kernel test robot <lkp@intel.com>,
	sfr@canb.auug.org.au,  llvm@lists.linux.dev,
	oe-kbuild-all@lists.linux.dev,
	 Christian Brauner <christianvanbrauner@gmail.com>,
	yangerkun <yangerkun@huawei.com>,
	 "zhangyi (F)" <yi.zhang@huawei.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	 linux-next@vger.kernel.org
Subject: Re: [brauner-vfs:vfs.misc 12/13] include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity.
Date: Tue, 23 Jan 2024 17:24:03 +0100	[thread overview]
Message-ID: <20240123-glatt-langgehegter-a239e588ae2c@brauner> (raw)
In-Reply-To: <59fae3eb-a125-cd5f-224e-b89d122ecb46@huawei.com>

On Tue, Jan 23, 2024 at 03:07:50PM +0800, Baokun Li wrote:
> On 2024/1/23 8:12, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.misc
> > head:   297983dc9011461cba6278bfe03f4305c4a2caa0
> > commit: 4bbd51d0f0ad709c0f02c100439d6c9ba6811d4b [12/13] fs: make the i_size_read/write helpers be smp_load_acquire/store_release()
> > config: i386-randconfig-015-20240123 (https://download.01.org/0day-ci/archive/20240123/202401230837.TXro0PHi-lkp@intel.com/config)
> > compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240123/202401230837.TXro0PHi-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202401230837.TXro0PHi-lkp@intel.com/
> > 
> > All errors (new ones prefixed by >>):
> > 
> >     In file included from fs/netfs/buffered_read.c:10:
> >     In file included from fs/netfs/internal.h:9:
> >     In file included from include/linux/seq_file.h:12:
> > > > include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity.
> >       911 |         return smp_load_acquire(&inode->i_size);
> >           |                ^
> >     include/asm-generic/barrier.h:206:2: note: expanded from macro 'smp_load_acquire'
> >       206 |         compiletime_assert_atomic_type(*p);                             \
> >           |         ^
> >     include/linux/compiler_types.h:438:2: note: expanded from macro 'compiletime_assert_atomic_type'
> >       438 |         compiletime_assert(__native_word(t),                            \
> >           |         ^
> >     include/linux/compiler_types.h:435:2: note: expanded from macro 'compiletime_assert'
> >       435 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >           |         ^
> >     include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
> >       423 |         __compiletime_assert(condition, msg, prefix, suffix)
> >           |         ^
> >     include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
> >       416 |                         prefix ## suffix();                             \
> >           |                         ^
> >     <scratch space>:38:1: note: expanded from here
> >        38 | __compiletime_assert_207
> >           | ^
> >     1 error generated.
> > --
> >     In file included from fs/netfs/buffered_write.c:9:
> > > > include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity.
> >       911 |         return smp_load_acquire(&inode->i_size);
> >           |                ^
> >     include/asm-generic/barrier.h:206:2: note: expanded from macro 'smp_load_acquire'
> >       206 |         compiletime_assert_atomic_type(*p);                             \
> >           |         ^
> >     include/linux/compiler_types.h:438:2: note: expanded from macro 'compiletime_assert_atomic_type'
> >       438 |         compiletime_assert(__native_word(t),                            \
> >           |         ^
> >     include/linux/compiler_types.h:435:2: note: expanded from macro 'compiletime_assert'
> >       435 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >           |         ^
> >     include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
> >       423 |         __compiletime_assert(condition, msg, prefix, suffix)
> >           |         ^
> >     include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
> >       416 |                         prefix ## suffix();                             \
> >           |                         ^
> >     <scratch space>:253:1: note: expanded from here
> >       253 | __compiletime_assert_207
> >           | ^
> >     In file included from fs/netfs/buffered_write.c:9:
> > > > include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity.
> >     include/asm-generic/barrier.h:206:2: note: expanded from macro 'smp_load_acquire'
> >       206 |         compiletime_assert_atomic_type(*p);                             \
> >           |         ^
> >     include/linux/compiler_types.h:438:2: note: expanded from macro 'compiletime_assert_atomic_type'
> >       438 |         compiletime_assert(__native_word(t),                            \
> >           |         ^
> >     include/linux/compiler_types.h:435:2: note: expanded from macro 'compiletime_assert'
> >       435 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >           |         ^
> >     include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
> >       423 |         __compiletime_assert(condition, msg, prefix, suffix)
> >           |         ^
> >     include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
> >       416 |                         prefix ## suffix();                             \
> >           |                         ^
> >     <scratch space>:253:1: note: expanded from here
> >       253 | __compiletime_assert_207
> >           | ^
> >     In file included from fs/netfs/buffered_write.c:9:
> > > > include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity.
> >     include/asm-generic/barrier.h:206:2: note: expanded from macro 'smp_load_acquire'
> >       206 |         compiletime_assert_atomic_type(*p);                             \
> >           |         ^
> >     include/linux/compiler_types.h:438:2: note: expanded from macro 'compiletime_assert_atomic_type'
> >       438 |         compiletime_assert(__native_word(t),                            \
> >           |         ^
> >     include/linux/compiler_types.h:435:2: note: expanded from macro 'compiletime_assert'
> >       435 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >           |         ^
> >     include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
> >       423 |         __compiletime_assert(condition, msg, prefix, suffix)
> >           |         ^
> >     include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
> >       416 |                         prefix ## suffix();                             \
> >           |                         ^
> >     <scratch space>:253:1: note: expanded from here
> >       253 | __compiletime_assert_207
> >           | ^
> >     In file included from fs/netfs/buffered_write.c:9:
> > > > include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity.
> >     include/asm-generic/barrier.h:206:2: note: expanded from macro 'smp_load_acquire'
> >       206 |         compiletime_assert_atomic_type(*p);                             \
> >           |         ^
> >     include/linux/compiler_types.h:438:2: note: expanded from macro 'compiletime_assert_atomic_type'
> >       438 |         compiletime_assert(__native_word(t),                            \
> >           |         ^
> >     include/linux/compiler_types.h:435:2: note: expanded from macro 'compiletime_assert'
> >       435 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >           |         ^
> >     include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
> >       423 |         __compiletime_assert(condition, msg, prefix, suffix)
> >           |         ^
> >     include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
> >       416 |                         prefix ## suffix();                             \
> >           |                         ^
> >     <scratch space>:253:1: note: expanded from here
> >       253 | __compiletime_assert_207
> >           | ^
> >     4 errors generated.
> > --
> >     In file included from fs/netfs/misc.c:8:
> >     In file included from include/linux/swap.h:9:
> >     In file included from include/linux/memcontrol.h:13:
> >     In file included from include/linux/cgroup.h:17:
> > > > include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity.
> >       911 |         return smp_load_acquire(&inode->i_size);
> >           |                ^
> >     include/asm-generic/barrier.h:206:2: note: expanded from macro 'smp_load_acquire'
> >       206 |         compiletime_assert_atomic_type(*p);                             \
> >           |         ^
> >     include/linux/compiler_types.h:438:2: note: expanded from macro 'compiletime_assert_atomic_type'
> >       438 |         compiletime_assert(__native_word(t),                            \
> >           |         ^
> >     include/linux/compiler_types.h:435:2: note: expanded from macro 'compiletime_assert'
> >       435 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >           |         ^
> >     include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
> >       423 |         __compiletime_assert(condition, msg, prefix, suffix)
> >           |         ^
> >     include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
> >       416 |                         prefix ## suffix();                             \
> >           |                         ^
> >     <scratch space>:251:1: note: expanded from here
> >       251 | __compiletime_assert_207
> >           | ^
> >     1 error generated.
> > 
> > Kconfig warnings: (for reference only)
> >     WARNING: unmet direct dependencies detected for DRM_I915_DEBUG_GEM
> >     Depends on [n]: HAS_IOMEM [=y] && DRM_I915 [=y] && EXPERT [=y] && DRM_I915_WERROR [=n]
> >     Selected by [y]:
> >     - DRM_I915_DEBUG [=y] && HAS_IOMEM [=y] && DRM_I915 [=y] && EXPERT [=y] && !COMPILE_TEST [=n]
> > 
> > 
> > vim +911 include/linux/fs.h
> > 
> >     874	
> >     875	void filemap_invalidate_lock_two(struct address_space *mapping1,
> >     876					 struct address_space *mapping2);
> >     877	void filemap_invalidate_unlock_two(struct address_space *mapping1,
> >     878					   struct address_space *mapping2);
> >     879	
> >     880	
> >     881	/*
> >     882	 * NOTE: in a 32bit arch with a preemptable kernel and
> >     883	 * an UP compile the i_size_read/write must be atomic
> >     884	 * with respect to the local cpu (unlike with preempt disabled),
> >     885	 * but they don't need to be atomic with respect to other cpus like in
> >     886	 * true SMP (so they need either to either locally disable irq around
> >     887	 * the read or for example on x86 they can be still implemented as a
> >     888	 * cmpxchg8b without the need of the lock prefix). For SMP compiles
> >     889	 * and 64bit archs it makes no difference if preempt is enabled or not.
> >     890	 */
> >     891	static inline loff_t i_size_read(const struct inode *inode)
> >     892	{
> >     893	#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> >     894		loff_t i_size;
> >     895		unsigned int seq;
> >     896	
> >     897		do {
> >     898			seq = read_seqcount_begin(&inode->i_size_seqcount);
> >     899			i_size = inode->i_size;
> >     900		} while (read_seqcount_retry(&inode->i_size_seqcount, seq));
> >     901		return i_size;
> >     902	#elif BITS_PER_LONG==32 && defined(CONFIG_PREEMPTION)
> >     903		loff_t i_size;
> >     904	
> >     905		preempt_disable();
> >     906		i_size = inode->i_size;
> >     907		preempt_enable();
> >     908		return i_size;
> >     909	#else
> >     910		/* Pairs with smp_store_release() in i_size_write() */
> >   > 911		return smp_load_acquire(&inode->i_size);
> >     912	#endif
> >     913	}
> >     914	
> Sorry to cause this compilation error, I have not encountered this
> before when testing compilation with gcc on x86 and arm64.
> Is there a special config required for this, or is it just clang that
> triggers this compile error?
> 
> The compile error seems to be that some architectural implementations
> of smp_load_acquire() do not support pointers to long long data
> types. Can't think of a good way to avoid this problem, any ideas
> from linus and christian?

That code in i_size_{read,write}() is terrible to look at.
Pragmatically, we can probably just do READ_ONCE() + smp_rmb() and
smp_wmb() + WRITE_ONCE(). This guarantees the ordering and it works
(compiles) on 32bit as well. I think it's still possible that on 32bit
the READ_ONCE()/WRITE_ONCE() are compiled as two accesses. But I'm not
sure that this matters because iiuc that problem would've already been
there with the barrier in mm/filemap.c instead of here.

  reply	other threads:[~2024-01-23 16:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <202401230837.TXro0PHi-lkp@intel.com>
2024-01-23  7:07 ` [brauner-vfs:vfs.misc 12/13] include/linux/fs.h:911:9: error: call to '__compiletime_assert_207' declared with 'error' attribute: Need native word sized stores/loads for atomicity Baokun Li
2024-01-23 16:24   ` Christian Brauner [this message]
2024-01-24  7:52     ` Baokun Li
2024-01-24 10:30       ` Christian Brauner
2024-01-24 11:53         ` Baokun Li
2024-01-24 13:27           ` Christian Brauner
2024-01-24 13:38             ` Baokun Li
2024-01-23 17:46   ` Linus Torvalds
2024-01-24  7:31     ` Baokun Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240123-glatt-langgehegter-a239e588ae2c@brauner \
    --to=brauner@kernel.org \
    --cc=christianvanbrauner@gmail.com \
    --cc=libaokun1@huawei.com \
    --cc=linux-next@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=llvm@lists.linux.dev \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=sfr@canb.auug.org.au \
    --cc=torvalds@linux-foundation.org \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox