Re: [PATCH 2/4] brd: extend the rcu regions to cover read and write

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

 



>   * Look up and return a brd's page for a given sector.
> + * This must be called with the rcu lock held.

Please ad a rcu_read_lock_held() check then.

> -	rcu_read_lock();
>  	idx = sector >> PAGE_SECTORS_SHIFT; /* sector to page index */
>  	page = radix_tree_lookup(&brd->brd_pages, idx);
> -	rcu_read_unlock();
> -
> -	BUG_ON(page && page->index != idx);
>  
>  	return page;

No need for the page variable now.  In fact there is no real need
for this helper now, as all the callers really should operate on
the sector on the index anyway.

>  }
> @@ -88,7 +74,9 @@ static bool brd_insert_page(struct brd_d
>  	struct page *page;
>  	gfp_t gfp_flags;
>  
> +	rcu_read_lock();
>  	page = brd_lookup_page(brd, sector);
> +	rcu_read_unlock();
>  	if (page)
>  		return true;

So this looks odd, as we drop the rcu lock without doing anything,
but it actually turns out to be correct as brd_do_bvec does yet
another lookup of it.  So we get an initial look, and optional
insert and then another lookup.  Not very efficient and it might be
worth to fix brd_do_bvec up to avoid these extra lookups given
that you touch it anyway (as would be an radix tree to xarray
conversion).




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux