linux-ext4 - Re: [PATCH] jbd/2[stable only]: Use WRITE_SYNC_PLUG in journal_commit

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]

Date:	Thu, 14 Jul 2011 16:01:00 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Jan Kara <jack@...e.cz>
Cc:	Jeff Moyer <jmoyer@...hat.com>, Tao Ma <tm@....ma>,
	linux-ext4@...r.kernel.org, stable@...nel.org,
	Corrado Zoccolo <czoccolo@...il.com>,
	Jens Axboe <jaxboe@...ionio.com>
Subject: Re: [PATCH] jbd/2[stable only]: Use WRITE_SYNC_PLUG in
 journal_commit_transaction.

On Thu, Jul 14, 2011 at 09:46:57PM +0200, Jan Kara wrote:
> On Thu 14-07-11 12:30:32, Jeff Moyer wrote:
> > Tao Ma <tm@....ma> writes:
> > >> - WRITE_SYNC_PLUG will plug the queue and expects explicity unplug. Who
> > >>   is doing unplug in this case?
> > > See the comments I removed, "we rely on sync_buffer() doing the unplug
> > > for us". I removed them cause we all use pluged write now.
> > 
> > Your logic is upside-down.  The code currently only uses the _PLUG
> > variant when t_synchronous_commit is set, meaning somebody *will* call
> > sync_buffer.  Simply setting WRITE_SYNC_PLUG doens't mean the upper
> > layer is going to issue the unplug.  Of course, I'm not 100% sure of the
> > journaling process, so it may very well be that there always is an
> > unplug.  Can Jan or someone comment on that?  Anyway, you could test
> > this theory by seeing if your kernel generates any timer unplugs in the
> > blktrace output.
>   So I'm not expert in plugging code but from what I understand when we do
> wait_on_buffer() (which calls io_schedule()) which will do
> blk_flush_plug()), the queue will get unplugged and IO starts. And we wait
> for all buffers we submit so we are guaranteed wait_on_buffer() will be
> called...

But blk_flush_plug() is called only in recent kernels where problem is
not present anyway.

Tao is reporting problem in 2.6.38 and 2.6.39. My concern is that if we
send all the IO as WRITE_SYNC_UNPLUG and not really unplug the queue
explicitly then we might lose more time in waiting for timer unplugs
and not benefit that much from merging.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html