2014-07-29 18:27 GMT+04:00 Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>:
> Andrey Ryabinin wrote:
>> On 07/29/14 15:33, Kirill A. Shutemov wrote:
>> > Things can go wrong if fault_around_bytes will be changed under
>> > do_fault_around(): between fault_around_mask() and fault_around_pages().
>> >
>> > Let's read fault_around_bytes only once during do_fault_around() and
>> > calculate mask based on the reading.
>> >
>> > Note: fault_around_bytes can only be updated via debug interface. Also
>> > I've tried but was not able to trigger a bad behaviour without the
>> > patch. So I would not consider this patch as urgent.
>> >
>> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
>> > ---
>> > mm/memory.c | 17 +++++++++++------
>> > 1 file changed, 11 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/mm/memory.c b/mm/memory.c
>> > index 9d66bc66f338..2ce07dc9b52b 100644
>> > --- a/mm/memory.c
>> > +++ b/mm/memory.c
>> > @@ -2772,12 +2772,12 @@ static unsigned long fault_around_bytes = rounddown_pow_of_two(65536);
>> >
>> > static inline unsigned long fault_around_pages(void)
>> > {
>> > - return fault_around_bytes >> PAGE_SHIFT;
>> > + return ACCESS_ONCE(fault_around_bytes) >> PAGE_SHIFT;
>> > }
>> >
>> > -static inline unsigned long fault_around_mask(void)
>> > +static inline unsigned long fault_around_mask(unsigned long nr_pages)
>> > {
>> > - return ~(fault_around_bytes - 1) & PAGE_MASK;
>> > + return ~(nr_pages * PAGE_SIZE - 1) & PAGE_MASK;
>> > }
>> >
>> >
>> > @@ -2844,12 +2844,17 @@ late_initcall(fault_around_debugfs);
>> > static void do_fault_around(struct vm_area_struct *vma, unsigned long address,
>> > pte_t *pte, pgoff_t pgoff, unsigned int flags)
>> > {
>> > - unsigned long start_addr;
>> > + unsigned long start_addr, nr_pages;
>> > pgoff_t max_pgoff;
>> > struct vm_fault vmf;
>> > int off;
>> >
>> > - start_addr = max(address & fault_around_mask(), vma->vm_start);
>> > + nr_pages = fault_around_pages();
>> > + /* race with fault_around_bytes_set() */
>> > + if (nr_pages <= 1)
>>
>> unlikely() ?
>
> Yep.
>
Btw, do we need this check at all? nr_pages can't be 0, and code below
seems able to handle nr_page == 1.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>