All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Tino Lehnig <tino.lehnig@contabo.de>,
	stable@vger.kernel.org, Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH 1/2] zram: remove BD_CAP_SYNCHRONOUS_IO with writeback feature
Date: Fri, 3 Aug 2018 12:00:19 +0900	[thread overview]
Message-ID: <20180803030019.GB86818@rodete-desktop-imager.corp.google.com> (raw)
In-Reply-To: <20180803023929.GA7500@jagdpanzerIV>

Hi Sergey,

On Fri, Aug 03, 2018 at 11:39:29AM +0900, Sergey Senozhatsky wrote:
> On (08/02/18 14:13), Andrew Morton wrote:
> [..]
> > That changelog is rather hard to follow.  Please review my edits:
> > 
> > : If zram supports writeback feature, it's no longer a BD_CAP_SYNCHRONOUS_IO
> 							^BDI_CAP_SYNCHRONOUS_IO
> 
> [..]
> 
> > A reader looking at this would wonder "why the heck are we doing that".
> > Adding a code comment would help them.
> 
> The interesting thing here is that include/linux/backing-dev.h
> BDI_CAP_SYNCHRONOUS_IO comment says
> 
> 	"Device is so fast that asynchronous IO would be inefficient."
> 
> Which is not the reason why BDI_CAP_SYNCHRONOUS_IO is used by ZRAM.
> Probably, the comment needs to be updated as well.

I couldn't catch your point. Could you clarify a little bit more?
What do you want to correct for the comment?

> 
> Both SWP_SYNCHRONOUS_IO and BDI_CAP_SYNCHRONOUS_IO tend to pivot
> "efficiency" [looking at the comments], but in ZRAM's case the whole
> reason to use SYNC IO is a race condition and user-after-free that
> follows.

Actually, it's not whole reason. As I wrote down, without it, swap_readpage
waits the IO completion for a long time by blk_poll so it causes system
sluggish problem when device is slow(e.g., zram with backing device).

Thanks.

  parent reply	other threads:[~2018-08-03  3:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-02  5:11 [PATCH 1/2] zram: remove BD_CAP_SYNCHRONOUS_IO with writeback feature Minchan Kim
2018-08-02 21:13 ` Andrew Morton
2018-08-03  2:39   ` Sergey Senozhatsky
2018-08-03  2:52     ` Sergey Senozhatsky
2018-08-03  3:00     ` Minchan Kim [this message]
2018-08-03  4:13       ` Sergey Senozhatsky
2018-08-03  4:51         ` Minchan Kim
2018-08-03  5:23           ` Sergey Senozhatsky
2018-08-03  5:45             ` Sergey Senozhatsky
2018-08-03  2:51   ` Minchan Kim
2018-08-03 23:28     ` Andrew Morton
2018-08-05 23:15       ` Minchan Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180803030019.GB86818@rodete-desktop-imager.corp.google.com \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=tino.lehnig@contabo.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.