Board index Linux Kernel

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by William Lee Irwin II » Tue, 04 Jun 2002 07:50:08



page->flags is effectively a lock word as its various bits are updatable
and accessible only by atomic operations. This patch removes the update
of page->flags in __free_pages_ok() with non-atomic operations in favor
of using atomic bit operations to update the bits to be cleared.

Against 2.5.19.

Cheers,
Bill

diff -Nru a/include/linux/page-flags.h b/include/linux/page-flags.h
--- a/include/linux/page-flags.h        Sun Jun  2 15:42:49 2002

 #define PageChecked(page)      test_bit(PG_checked, &(page)->flags)
 #define SetPageChecked(page)   set_bit(PG_checked, &(page)->flags)
+#define ClearPageChecked(page) clear_bit(PG_checked, &(page)->flags)

 #define PageReserved(page)     test_bit(PG_reserved, &(page)->flags)
 #define SetPageReserved(page)  set_bit(PG_reserved, &(page)->flags)
diff -Nru a/mm/page_alloc.c b/mm/page_alloc.c
--- a/mm/page_alloc.c   Sun Jun  2 15:42:49 2002

        BUG_ON(PageLRU(page));
        BUG_ON(PageActive(page));
        BUG_ON(PageWriteback(page));
+
        ClearPageDirty(page);
-       page->flags &= ~(1<<PG_referenced);
+       ClearPageUptodate(page);
+       ClearPageSlab(page);
+       ClearPageNosave(page);
+       ClearPageChecked(page);

        if (current->flags & PF_FREE_PAGES)
                goto local_freelist;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by Hugh Dickin » Tue, 04 Jun 2002 19:20:07



Quote:> page->flags is effectively a lock word as its various bits are updatable
> and accessible only by atomic operations. This patch removes the update
> of page->flags in __free_pages_ok() with non-atomic operations in favor
> of using atomic bit operations to update the bits to be cleared.

>    ClearPageDirty(page);
> -  page->flags &= ~(1<<PG_referenced);
> +  ClearPageUptodate(page);
> +  ClearPageSlab(page);
> +  ClearPageNosave(page);
> +  ClearPageChecked(page);

Don't all those atomic volatile bitops slow down a hotpath for no real
gain?  I'm all for clearing all possible flag bits at that point, but
would prefer just one mask myself.  But given all the preceding tests,
and the ClearPageDirty, perhaps I'm foolish to question your additions.

And wasn't it originally clearing the referenced bit, now leaving it?

Hugh

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by David S. Mille » Tue, 04 Jun 2002 19:20:10



   Date: Mon, 3 Jun 2002 11:14:43 +0100 (BST)


   >
   >         ClearPageDirty(page);
   > -       page->flags &= ~(1<<PG_referenced);
   > +       ClearPageUptodate(page);
   > +       ClearPageSlab(page);
   > +       ClearPageNosave(page);
   > +       ClearPageChecked(page);

   Don't all those atomic volatile bitops slow down a hotpath for no real
   gain?  I'm all for clearing all possible flag bits at that point, but
   would prefer just one mask myself.  But given all the preceding tests,
   and the ClearPageDirty, perhaps I'm foolish to question your additions.

   And wasn't it originally clearing the referenced bit, now leaving it?

Furthermore, when this code runs there should be no way for any
part of the kernel to reference the page in a way that cares
about these flag bits.

So if anything, we should be doing _ALL_ of the flag bitsops
non-atomically.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by William Lee Irwin II » Tue, 04 Jun 2002 19:40:06



>> page->flags is effectively a lock word as its various bits are updatable
>> and accessible only by atomic operations. This patch removes the update
>> of page->flags in __free_pages_ok() with non-atomic operations in favor
>> of using atomic bit operations to update the bits to be cleared.
>>        ClearPageDirty(page);
>> -      page->flags &= ~(1<<PG_referenced);
>> +      ClearPageUptodate(page);
>> +      ClearPageSlab(page);
>> +      ClearPageNosave(page);
>> +      ClearPageChecked(page);

> Don't all those atomic volatile bitops slow down a hotpath for no real
> gain?  I'm all for clearing all possible flag bits at that point, but
> would prefer just one mask myself.  But given all the preceding tests,
> and the ClearPageDirty, perhaps I'm foolish to question your additions.
> And wasn't it originally clearing the referenced bit, now leaving it?
> Hugh

It should be clearing it, I'd retransmit if there weren't other objections
to address...

Cheers,
Bill
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by David S. Mille » Tue, 04 Jun 2002 19:40:08



   Date: Mon, 3 Jun 2002 03:28:09 -0700

   It should be clearing it, I'd retransmit if there weren't other objections
   to address...

Such as the fact that none of these operations need to
be atomic :-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by William Lee Irwin II » Tue, 04 Jun 2002 20:10:05



Date: Mon, 3 Jun 2002 03:28:09 -0700

>    It should be clearing it, I'd retransmit if there weren't other objections
>    to address...

> Such as the fact that none of these operations need to
> be atomic :-)

After looking around a little bit the unique reference guarantee
plus various memory barriers surrounding it appear to suffice. But
I'm still left quite uneasy by the usage of non-atomic operations
on an atomically updated lock word. At the very least making the
operand shifted an unsigned long is needed. So this ensues:

Cheers,
Bill

===== mm/page_alloc.c 1.63 vs edited =====
--- 1.63/mm/page_alloc.c        Tue May 28 16:57:49 2002

                BUG();
        if (PageWriteback(page))
                BUG();
-       ClearPageDirty(page);
-       page->flags &= ~(1<<PG_referenced);
+
+       page->flags &= ~((1UL << PG_referenced) | (1UL << PG_dirty));

        if (current->flags & PF_FREE_PAGES)
                goto local_freelist;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by David S. Mille » Tue, 04 Jun 2002 20:10:07



   Date: Mon, 3 Jun 2002 04:00:55 -0700

        if (PageWriteback(page))
                BUG();
   -    ClearPageDirty(page);
   -    page->flags &= ~(1<<PG_referenced);
   +
   +    page->flags &= ~((1UL << PG_referenced) | (1UL << PG_dirty));

Umm, nevermind.  Look at ClearPageDirty, it does
"other stuff" so you can't remove it wholesale.

In the end, the code is as it should be right now.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

remove mixture of non-atomic operations with page->flags which requires atomic operations to access

Post by William Lee Irwin II » Tue, 04 Jun 2002 20:20:08



>    Date: Mon, 3 Jun 2002 04:00:55 -0700

>            if (PageWriteback(page))
>                    BUG();
>    -       ClearPageDirty(page);
>    -       page->flags &= ~(1<<PG_referenced);
>    +
>    +       page->flags &= ~((1UL << PG_referenced) | (1UL << PG_dirty));

> Umm, nevermind.  Look at ClearPageDirty, it does
> "other stuff" so you can't remove it wholesale.
> In the end, the code is as it should be right now.

Ugh, even if it isn't I don't care to deal with it, rusty, chuck this one.

Cheers,
Bill
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 
Top

1. atomic operations in solaris (atomic.h)

hi

I'm porting a server from NT to Solaris and need to replace
some 'Interlocked' operations.

I found some atomic functions in the file /sys/atomic.h which seem to
fit my needs very well.  But I can't find any implementation for these
functions... atomic_add_long and others.

In what lib file are they implemented ?  Or are they not user-level
function?

thanks.
--
Matthias Asgeirsson

Sent via Deja.com http://www.deja.com/
Before you buy.

2. K6-2 vs Duron - a server platform

3. Is it an atomic operation !!!???

4. Modifying lilo.conf on boot diskette

5. Solaris atomic/interlocked increment/swap operations?

6. nslookup trough VPN fails

7. Atomic/transaction operations on UFS

8. Sun Solaris Engineers Blogging

9. Atomic operations at Solaris

10. Atomic operations

11. Question about "atomic" operations

12. is file<<"STRING"<<endl.atomic operation.

13. Testing existance of process and killing it with one atomic operation..?


All times are UTC
Board index