From owner-svn-src-head@FreeBSD.ORG Wed Apr 20 22:06:24 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 451D3106564A; Wed, 20 Apr 2011 22:06:24 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.mail.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id 8648F8FC17; Wed, 20 Apr 2011 22:06:23 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApwEAHZYr02DaFvO/2dsb2JhbACEUaFtiG+tLJEOgSmBWYF1egSOIg X-IronPort-AV: E=Sophos;i="4.64,247,1301889600"; d="scan'208";a="118159455" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu-pri.mail.uoguelph.ca with ESMTP; 20 Apr 2011 18:06:23 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 0075CB3F5C; Wed, 20 Apr 2011 18:06:22 -0400 (EDT) Date: Wed, 20 Apr 2011 18:06:22 -0400 (EDT) From: Rick Macklem To: Pawel Jakub Dawidek Message-ID: <558114287.379554.1303337182943.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20110420161655.GA1907@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.202] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - IE7 (Win)/6.0.10_GA_2692) Cc: svn-src-head@freebsd.org, Rick Macklem , svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r220877 - head/sys/fs/nfsclient X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Apr 2011 22:06:24 -0000 > > This is not that easy, I'm afraid. You need to ask yourself a question > what you are trying to protect from. Here, the mutex only guarantees > to > have consistent view of the nm_maxfilesize field. For example if this > field modification wouldn't be atomic you would need the mutex to > ensure > that the value is correct. > > Imagine a situation where it is modifed not by simple 'a = b', but by > something like this: > > mtx_lock(&nmp->nm_mtx); > nmp->nm_maxfilesize = some_32bit_array[0]; > nmp->nm_maxfilesize |= some_32bit_array[1] << 32; > mtx_unlock(&nmp->nm_mtx); > > To read that properly you need a mutex to ensure you won't read the > value between those two operations. > > If it is not the case - its modification is atomic and reading it is > atomic then you don't need mutex to read the value as it will always > be > consistent. Well, its value will be consistent, but not necessarily the "up to date" value set by another thread, if I understood alc@'s recent post. If you haven't yet read it, take a look at his post today on freebsd-hackers@ under the Subject Re: SMP question w.r.t. reading kernel variables If I understood his explanation, a thread must lock a mutex that the thread that modified the value has locked/unlocked before it is guaranteed to see that value for the variable instead of some stale memory cached value. (It can be any mutex, but it is probably much easier to use the same one that the modifying thread used during modification instead of finding some other mutex the same thread locked/unlocked after the modification.) > The question is what will happen if it changes after you > read it. > > thread0 thread1 > ------- ------- > > mtx_lock(&nmp->nm_mtx); > nmp->nm_maxfilesize = 8192; > mtx_unlock(&nmp->nm_mtx); > > mtx_lock(&nmp->nm_mtx); > if (tmp_off > nmp->nm_maxfilesize) { > mtx_unlock(&nmp->nm_mtx); > return (EFBIG); > } > mtx_unlock(&nmp->nm_mtx); > > mtx_lock(&nmp->nm_mtx); > nmp->nm_maxfilesize = 2048; > mtx_unlock(&nmp->nm_mtx); > > > > Now, if tmp_off is 4096 what will happen if you have a race like the > above? Is it critical? Then you need to protect with > this mutex as well. > Yes, understood. I need to look at the above check w.r.t. nm_maxfilesize further (such as discussed by bde@). If a server were to change the value during a read or write, all that should happen is that the server will return an error for an operation that goes past its specified maxfilesize. (As you noted, it is unlikely to ever change and, if it did change, I suspect the server would "grow" it and not "shrink" it.) The mtx_lock(&nmp->nm_mtx); and mtx_unlock(&nmp->nm_mtx); just ensures that it reads the "most up to date" value for the variable set by another thread. rick