Re: [PATCH 4/4] hfsplus: get rid of write_super

From: Artem Bityutskiy
Date: Thu Jun 21 2012 - 16:30:56 EST


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

Attachment: signature.asc
Description: This is a digitally signed message part