linux-kernel - Re: [PATCH 4/4] hfsplus: get rid of write

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: <1340310656.2536.12.camel@koala>
Date:	Thu, 21 Jun 2012 23:30:56 +0300
From:	Artem Bityutskiy <dedekind1@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Al Viro <viro@...IV.linux.org.uk>,
	Linux FS Maling List <linux-fsdevel@...r.kernel.org>,
	Linux Kernel Maling List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] hfsplus: get rid of write_super

On Thu, 2012-06-21 at 12:41 -0700, Andrew Morton wrote:
> > +	spin_lock(&sbi->work_lock);
> > +	if (!sbi->work_queued) {
> > +	       delay = msecs_to_jiffies(dirty_writeback_interval * 10);
> > +	       queue_delayed_work(system_long_wq, &sbi->sync_work, delay);
> > +	       sbi->work_queued = 1;
> > +	}
> > +	spin_unlock(&sbi->work_lock);
> >  }
> 
> And I think it could be made to go away here, perhaps by switching to
> test_and_set_bit or similar.

Yes, you are right, and I thought about this. But I did not want to make
it more complicated than needed. Spinlock is simple and
straight-forward, this is not a fast-path.

> And I wonder about the queue_delayed_work().  iirc this does nothing to
> align timer expiries, so someone who has a lot of filesystems could end
> up with *more* timer wakeups.  Shouldn't we do something here to make
> the system do larger amounts of work per timer expiry?  Such as the
> timer-slack infrastructure?

Well, I also thought about this. But I again did not want to invent
anything complex because main file systems - ext4, btrfs, xfs - do not
use 'write_super()' at all. And then only these dying / rare
file-systems like btrfs / hfs - I did not feel like over-engineering is
needed.

If someone is affected by more timers, which I really really doubt, we
can further optimize this using deferrable timers for some file-systems
which do not really think anything super-important, and we can use range
hrtimers for file-systems which sync something more or less important.
The former is easy to do, the latter would need improving the workqueue
infrastructure.

> It strikes me that this whole approach improves the small system with
> little write activity, but makes things worse for the larger system
> with a lot of filesystems?

Well, if we have a lot of those rare FSes and do not have much
activities, then we do not wake up. If we have a lot of activities,
probably it does not hurt to wake-up much. But of course there is a
situation when this would hurt, but I again, do doubt such systems are
common, an really care.

IOW, I do not dismiss your point, but I think that we rather see if it
affects anyone and then optimize with a deferrable/range timers.

-- 
Best Regards,
Artem Bityutskiy

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)