Re: [PATCH 2/9] device-mapper log bitset: fix endian

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 22, 2006 at 09:37:41PM -0800, Andrew Morton wrote:
> Alasdair G Kergon <[email protected]> wrote:
> >
> >  -	set_bit(bit, (unsigned long *) bs);
> >  +	ext2_set_bit(bit, (unsigned long *) bs);
 
> ext2_set_bit() is non-atomic, so the above code must provide its own
> locking against other CPUs (and threads, if preempt) performing
> modification of this memory.
> 
> Is such locking present?  If not, we should use ext2_set_bit_atomic(). 
> (And if so, the old code could have used __set_bit)

As far as I can tell, all the sets and clears happen from the same 
single-threaded workqueue, but one mirror_map() could run alongside:

        r = ms->rh.log->type->in_sync(ms->rh.log,
                                      bio_to_region(&ms->rh, bio), 0);

which uses:
        ext2_test_bit(bit, (unsigned long *) bs) ? 1 : 0;

So far I haven't found any races here that would cause problems.
(And if there are any, they're probably wider than just making
those operations atomic.)

And it also looks like this code doesn't handle barriers correctly...

Alasdair
-- 
[email protected]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux