Bug 7602

On Thu, 30 Nov 2006 13:18:53 -0800
bugme-daemon@bugzilla.kernel.org wrote:

> 
> http://bugzilla.kernel.org/show_bug.cgi?id=7602
> 
>            Summary: Failure on compilation: include/asm/bitops.h:122: error:
>                     inconsistent operand constraints in an `asm' in
>                     nfs_access_add_cache()
>     Kernel Version: 2.6.19
>             Status: NEW
>           Severity: high
>              Owner: trond.myklebust@fys.uio.no
>          Submitter: mjw99@ic.ac.uk
> 
> 
> Most recent kernel where this bug did *NOT* occur: 
> 2.6.18.3
> 
> Distribution: 
> RHEL4-U2 AS
> 
> Hardware Environment:
> 8 CPU Dual Core Opteron box with 32Gb of ram 
> http://www.iwill.net/product_2.asp?p_id=90&sp=Y
> 
> Software Environment:
> RHEL4-U2
> gcc version 3.4.4 20050721 (Red Hat 3.4.4-2)
> 
> Problem Description:
> 
> Compilation fails with the error message:
> ....
>   CC      fs/nfs/client.o
>   CC      fs/nfs/dir.o
> fs/nfs/dir.c: In function `nfs_access_add_cache':
> include/asm/bitops.h:122: error: inconsistent operand constraints in an `asm'

Seems that this:

static __inline__ int __test_and_set_bit(int nr, volatile void * addr)
{
	int oldbit;

	__asm__(
		"btsl %2,%1\n\tsbbl %0,%0"
		:"=r" (oldbit),"+m" (ADDR)
		:"dIr" (nr));
	return oldbit;
}

explodes with gcc-3.4.4.

> 
> static __inline__ int __test_and_set_bit(int nr, volatile void * addr)
> {
> 	int oldbit;
> 
> 	__asm__(
> 		"btsl %2,%1\n\tsbbl %0,%0"
> 		:"=r" (oldbit),"+m" (ADDR)
> 		:"dIr" (nr));
> 	return oldbit;
> }
> 
> explodes with gcc-3.4.4.

Known issue.  The new form is correct and needed, but the old gcc doesn't accept
it. I haven't gotten a form that is both and correct and works on the old compiler
out of the gcc hackers I asked.

Probably need to #ifdef it.

-Andi

On Thu, 30 Nov 2006 23:22:00 +0100
Andi Kleen <ak@suse.de> wrote:

> > 
> > static __inline__ int __test_and_set_bit(int nr, volatile void * addr)
> > {
> > 	int oldbit;
> > 
> > 	__asm__(
> > 		"btsl %2,%1\n\tsbbl %0,%0"
> > 		:"=r" (oldbit),"+m" (ADDR)
> > 		:"dIr" (nr));
> > 	return oldbit;
> > }
> > 
> > explodes with gcc-3.4.4.
> 
> Known issue.  The new form is correct and needed, but the old gcc doesn't accept
> it. I haven't gotten a form that is both and correct and works on the old compiler
> out of the gcc hackers I asked.

Oh, thanks.

What does "d" do, btw?  My gcc info page only covers "x86" and says only "`d' register"

(And, more importantly, where is the best description of gcc asm constraints?)

On Thursday 30 November 2006 23:32, Andrew Morton wrote:
> On Thu, 30 Nov 2006 23:22:00 +0100
> Andi Kleen <ak@suse.de> wrote:
> 
> > > 
> > > static __inline__ int __test_and_set_bit(int nr, volatile void * addr)
> > > {
> > > 	int oldbit;
> > > 
> > > 	__asm__(
> > > 		"btsl %2,%1\n\tsbbl %0,%0"
> > > 		:"=r" (oldbit),"+m" (ADDR)
> > > 		:"dIr" (nr));
> > > 	return oldbit;
> > > }
> > > 
> > > explodes with gcc-3.4.4.
> > 
> > Known issue.  The new form is correct and needed, but the old gcc doesn't accept
> > it. I haven't gotten a form that is both and correct and works on the old compiler
> > out of the gcc hackers I asked.
> 
> Oh, thanks.
> 
> What does "d" do, btw?  My gcc info page only covers "x86" and says only "`d' register"

Hmm, normally edx (aka Extended D register eXtended :) or rdx 

But you're right it doesn't make sense here because 'd' is already included in 'r'.
Probably should be dropped.

> 
> (And, more importantly, where is the best description of gcc asm constraints?)

Either info pages or gcc source. There was also a web page somewhere with a tutorial,
but i don't think it was a full reference.

-Andi

Reply-To: randy.dunlap@oracle.com

On Thu, 30 Nov 2006 23:49:12 +0100 Andi Kleen wrote:

> On Thursday 30 November 2006 23:32, Andrew Morton wrote:
> > On Thu, 30 Nov 2006 23:22:00 +0100
> > Andi Kleen <ak@suse.de> wrote:
> > 
> > > > 
> > > > static __inline__ int __test_and_set_bit(int nr, volatile void * addr)
> > > > {
> > > > 	int oldbit;
> > > > 
> > > > 	__asm__(
> > > > 		"btsl %2,%1\n\tsbbl %0,%0"
> > > > 		:"=r" (oldbit),"+m" (ADDR)
> > > > 		:"dIr" (nr));
> > > > 	return oldbit;
> > > > }
> > > > 
> > > > explodes with gcc-3.4.4.
> > > 
> > > Known issue.  The new form is correct and needed, but the old gcc doesn't accept
> > > it. I haven't gotten a form that is both and correct and works on the old compiler
> > > out of the gcc hackers I asked.
> > 
> > Oh, thanks.
> > 
> > What does "d" do, btw?  My gcc info page only covers "x86" and says only "`d' register"
> 
> Hmm, normally edx (aka Extended D register eXtended :) or rdx 
> 
> But you're right it doesn't make sense here because 'd' is already included in 'r'.
> Probably should be dropped.
> 
> > 
> > (And, more importantly, where is the best description of gcc asm constraints?)
> 
> Either info pages or gcc source. There was also a web page somewhere with a tutorial,
> but i don't think it was a full reference.

e.g. (may be ix86 instead of x86_64):

http://gcc.gnu.org/onlinedocs/gcc-4.1.1/gcc/Constraints.html#Constraints

http://www-128.ibm.com/developerworks/linux/library/l-ia.html

http://www.uwsg.indiana.edu/hypermail/linux/kernel/9804.2/0953.html

---
~Randy

I am getting a very similar error in 2.6.19.1 with gcc 3.4.6 on a duo core
Athlon64.  However, it seems to be the atomic version here:

static __inline__ int test_and_set_bit(int nr, volatile void * addr)
{
	int oldbit;

	__asm__ __volatile__( LOCK_PREFIX
		"btsl %2,%1\n\tsbbl %0,%0"
		:"=r" (oldbit),"+m" (ADDR)
		:"dIr" (nr) : "memory");
	return oldbit;
}

Is there a work around at all for this?  I can not get NFS to compile in the
kernel currently.

Thanks

I can confirm that the problem also occurs under 2.6.19.1 and gcc 3.4.6 on
x86_64 with smp.

Andi Kleen writes : "Known issue.  The new form is correct and needed, but the
old gcc doesn't accept it."

But imo these gcc versions are rather new and being used by many people, so I
think it is NOT acceptable to use this "new form" if it is doesn't compile on
mainstream compilers.

For the moment I can't compile 2.6.19.(1) for our Athlons 64 if I need NFS
without hacking the kernel and that is really serious.

Please give this bug a high priority ?

thanks,
Kees Lemmens.

It is certainly out of the question to change generic (correct!) NFS code due to
a compile bug in test_and_set_bit().

I'm reassigning this bug as being an x86_64 platform issue.

A dirty reversal to the function as it was in 2.6.18.3:
1878a1879,1902
> //void nfs_access_add_cache(struct inode *inode, struct nfs_access_entry *set)
> //{
> //    struct nfs_access_entry *cache = kmalloc(sizeof(*cache), GFP_KERNEL);
> //    if (cache == NULL)
> //            return;
> //    RB_CLEAR_NODE(&cache->rb_node);
> //    cache->jiffies = set->jiffies;
> //    cache->cred = get_rpccred(set->cred);
> //    cache->mask = set->mask;
> //
> //    nfs_access_add_rbtree(inode, cache);
> //
> //    /* Update accounting */
> //    smp_mb__before_atomic_inc();
> //    atomic_long_inc(&nfs_access_nr_entries);
> //    smp_mb__after_atomic_inc();
> //
> //    /* Add inode to global LRU list */
> //    if (!test_and_set_bit(NFS_INO_ACL_LRU_SET, &NFS_FLAGS(inode))) {
> //            spin_lock(&nfs_access_lru_lock);
> //            list_add_tail(&NFS_I(inode)->access_cache_inode_lru,
&nfs_access_lru_list);
> //            spin_unlock(&nfs_access_lru_lock);
> //    }
> //}
1881,1889c1905,1906
<       struct nfs_access_entry *cache = kmalloc(sizeof(*cache), GFP_KERNEL);
<       if (cache == NULL)
<               return;
<       RB_CLEAR_NODE(&cache->rb_node);
<       cache->jiffies = set->jiffies;
<       cache->cred = get_rpccred(set->cred);
<       cache->mask = set->mask;
<
<       nfs_access_add_rbtree(inode, cache);
---
>       struct nfs_inode *nfsi = NFS_I(inode);
>       struct nfs_access_entry *cache = kmalloc(sizeof(*cache), GFP_KERNEL);//
&nfsi->cache_access;
1891,1900c1908,1911
<       /* Update accounting */
<       smp_mb__before_atomic_inc();
<       atomic_long_inc(&nfs_access_nr_entries);
<       smp_mb__after_atomic_inc();
<
<       /* Add inode to global LRU list */
<       if (!test_and_set_bit(NFS_INO_ACL_LRU_SET, &NFS_FLAGS(inode))) {
<               spin_lock(&nfs_access_lru_lock);
<               list_add_tail(&NFS_I(inode)->access_cache_inode_lru,
&nfs_access_lru_list);
<               spin_unlock(&nfs_access_lru_lock);
---
>       if (cache->cred != set->cred) {
>               if (cache->cred)
>                       put_rpccred(cache->cred);
>               cache->cred = get_rpccred(set->cred);
1901a1913,1918
>       /* FIXME: replace current access_cache BKL reliance with inode->i_lock */
>       spin_lock(&inode->i_lock);
>       nfsi->cache_validity &= ~NFS_INO_INVALID_ACCESS;
>       spin_unlock(&inode->i_lock);
>       cache->jiffies = set->jiffies;
>       cache->mask = set->mask;

This is my first attempt at playing with the kernel, so if this is silly please
disregard.

Created attachment 10039 [details]
This workaround helped me compile nfs with 3.4.3-22.1

Hi Adrian Roberts and others,

I tried your patch on our Slamd64 systems and 2.16.19.1 and it compiles fine and
boots, but crashes horribly in the NFS part ... So this doesn't work.

Installing gcc 4.1 for just one silly routine in one single file is out of the
question : then I have to replace almost the whole system (both gcc, and many
libraries) ! I don't see why the original author doesn't see this : it should
simply compile on mainstream GCC compilers, which are currently gcc 3.4 and if
not it should be changed asap. Point.

Is there anyone with a better solution ? At the time it seems to be impossible
to compile any new 2.6 kernel on most 64-bit distros only because of these few
silly lines added by Andi Kleen !

Kees Lemmens.

Hi Ravikiran Thirumalai,

You are fantastic !! It works !! Now 2.6.19.1 compiles fine with gcc 3.4, the
kernel boots fine and even NFS seems to be working without any problems !!
Thanks a lot for your efforts and for posting your solution !

Andi : can this little patch (only 2 lines !) be added to the kernel sources
asap ? This is a lot better than struggling for 3 days to install gcc 4.1 and
libc 4 only to find out that the majority of the code you already have installed
doesn't work anymore with the new libraries and that you have to compile
everything from scratch ...

This patch is the difference between 2 weeks of work and just 5 minutes ...

Bye,
Kees Lemmens.

Kees Lemmens,
As demonstrated by the patch this seems to be a gcc bug rather than a kernel
bug.    The right thing IMHO is to open a bug with gcc/your distro/ rather than
blame kernel developers.  This asm routine has been in the kernel earlier than
2.6.17, just that the nfs code started using this from 2.6.19 triggering this
breakage.

I submitted a workaround for this to mainline now

Seems bug is stale in linux-2.6.19.2

root@lion:/usr/src/linux-2.6.19.2# gcc -v
Reading specs from /usr/lib64/gcc/x86_64-slackware-linux/3.4.6/specs
Configured with: ../gcc-3.4.6/configure --prefix=/usr --disable-multilib --
libdir=/usr/lib64 --enable-shared --enable-threads=posix --enable-__cxa_atexit -
-disable-checking --with-gnu-ld --verbose --target=x86_64-slackware-linux --
host=x86_64-slackware-linux --build=x86_64-slackware-linux
Thread model: posix
gcc version 3.4.6


Where i can get the workaround?

This is fixed in Linus tree with commit
24420760c3701ff422b344e047a20ca09b76fc64. Thanks Andi!