lkml.org 
[lkml]   [2004]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Unnecessary barrier in sync_page()?
On Wed, Jul 07, 2004 at 02:57:24PM -0300, Marcelo Tosatti wrote:
>
> Hi Chris,
>
> I was talking to Andrew about this memory barrier
>
> static inline int sync_page(struct page *page)
> {
> struct address_space *mapping;
>
> /*
> * FIXME, fercrissake. What is this barrier here for?
> */
> smp_mb();
> mapping = page_mapping(page);
> if (mapping && mapping->a_ops && mapping->a_ops->sync_page)
> return mapping->a_ops->sync_page(page);
> return 0;
> }
>
> And does not seem to be a reason for it. The callers are:
>
> void fastcall wait_on_page_bit(struct page *page, int bit_nr)
> {
> wait_queue_head_t *waitqueue = page_waitqueue(page);
> DEFINE_PAGE_WAIT(wait, page, bit_nr);
>
> do {
> prepare_to_wait(waitqueue, &wait.wait, TASK_UNINTERRUPTIBLE);
> if (test_bit(bit_nr, &page->flags)) {
> sync_page(page);
> io_schedule();
> }
> } while (test_bit(bit_nr, &page->flags));
> finish_wait(waitqueue, &wait.wait);
> }
>
> void fastcall __lock_page(struct page *page)
> {
> wait_queue_head_t *wqh = page_waitqueue(page);
> DEFINE_PAGE_WAIT_EXCLUSIVE(wait, page, PG_locked);
>
> while (TestSetPageLocked(page)) {
> prepare_to_wait_exclusive(wqh, &wait.wait, TASK_UNINTERRUPTIBLE);
> if (PageLocked(page)) {
> sync_page(page);
> io_schedule();
> }
> }
> finish_wait(wqh, &wait.wait);
> }
>
> Both callers call set_bit (atomic operation which cannot be reordered) before

set_bit is atomic but it _can_ be reordered just fine. atomic !=
barrier (they're the same only in x86 due the lack of specific smp-aware
opcodes).

however the smp_mb() isn't needed in sync_page, simply because it's
perfectly ok if we start running sync_page before reading pagelocked.
All we care about is to run sync_page _before_ io_schedule() and that we
read PageLocked _after_ prepare_to_wait_exclusive.

So the locking in between PageLocked and sync_page is _absolutely_
worthless and the smp_mb() can go away.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:04    [W:0.092 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site