linux-kernel - Re: [PATCHv4 17/17] writeback: lessen sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1274957469.13159.20.camel@localhost>
Date:	Thu, 27 May 2010 13:51:09 +0300
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Nick Piggin <npiggin@...e.de>
Cc:	Al Viro <viro@...IV.linux.org.uk>,
	LKML <linux-kernel@...r.kernel.org>,
	Jens Axboe <jens.axboe@...cle.com>,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCHv4 17/17] writeback: lessen sync_supers wakeup count

Nick, thanks for serialization suggestion.

On Thu, 2010-05-27 at 17:22 +1000, Nick Piggin wrote:
> Yeah, we definitely don't want to add global cacheline writes in the
> common case. Also I don't know why you do the strange -1 value. I
> couldn't seem to find where you defined bdi_arm_supers_timer();

It is in mm/backing-dev.c:376 in today's Linus' tree. The -1 is used to
indicate that 'sync_supers()' is in progress and avoid arming timer in
that case. But yes, this is not really needed.

> But why doesn't this work?
> 
>   sb->s_dirty = 1;
>   smp_mb(); /* corresponding MB is in test_and_clear_bit */

AFAIU, test_and_clear_bit assumes 2 barriers - before the test and after
the clear. Then I do not really understand why this smp_mb is needed.

>   if (unlikely(!supers_timer_armed)) {
>     if (!test_and_set_bit(0, &supers_timer_armed))
>         bdi_arm_supers_timer();
>   }
>     
>   vs
> 
>   supers_timer_armed = 0;
> again:
>   sync_supers();
>   if (test_and_clear_bit(0, &supers_timer_armed))
>     goto again;

AFAIU, the following should be fine, no?:


    if (unlikely(!supers_timer_armed))
        if (!test_and_set_bit(0, &supers_timer_armed))
            bdi_arm_supers_timer();

vs

again:
    sync_supers();
    if (test_and_clear_bit(0, &supers_timer_armed))
        goto again;

I assume that it is harmless to run 'bdi_arm_supers_timer()'
concurrently;

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

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