On Mon, 2007-02-26 at 23:39 +0000, Keir Fraser wrote: > On 26/2/07 23:31, "Hollis Blanchard" <[EMAIL PROTECTED]> wrote: > > > > > Hi Yamahata-san, thanks very much for your patch! > > > > I'm confused about one thing though: why do we need to take a lock to > > read d->grant_table->nr_grant_frames? It's a simple integer, so no lock > > is required or useful. > > I haven't got the ppc code immediately to hand but the x86 code will > dynamically grow the grant table based on requests made via the > add_to_physmap hypercall, hence it needs to take the lock.
OK, I see x86's XENMAPSPACE_grant_table handler; the locking makes sense there. > I see ia64 takes > it also even though it only reads from nr_grant_frames (it won;t dynamically > expand via the add_to_physmap hypercall). That's possibly overkill although > there is some concern over reading nr_grant_frames versus looking up a > grant-table page that appears within the range [0, nr_grant_frames-1] which > taking the lock trivially sidesteps. If ia64 didn't take the lock it might > be possible to see an increased value of nr_grant_frames that the expand > function hadn't yet fully installed the new grant-table pages for yet. I don't believe that's a concern, since updating grant_table->nr_grant_frames is the very last step in gnttab_grow_table(), and it will only grow. The comments about locking around nr_grant_frames() and nr_grant_entries() are confusing at best, since you don't need a lock to read an integer... -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-ppc-devel mailing list Xenemail@example.com http://lists.xensource.com/xen-ppc-devel