Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Dan Streetman <ddstreet@ieee.org>,
	Nitin Gupta <ngupta@vflare.org>,
	Seth Jennings <sjenning@redhat.com>, Yu Zhao <yuzhao@google.com>,
	Linux-MM <linux-mm@kvack.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Dan Streetman <dan.streetman@canonical.com>
Subject: Re: [PATCH] mm/zsmalloc: don't fail if can't create debugfs info
Date: Fri, 29 Apr 2016 14:37:40 +0900
Message-ID: <20160429053740.GA2431@bbox> (raw)
In-Reply-To: <20160429003824.GC4920@swordfish>

On Fri, Apr 29, 2016 at 09:38:24AM +0900, Sergey Senozhatsky wrote:
> On (04/28/16 15:07), Andrew Morton wrote:
> > Needed a bit of tweaking due to
> > http://ozlabs.org/~akpm/mmotm/broken-out/zsmalloc-reordering-function-parameter.patch
> 
> Thanks.
> 
> > From: Dan Streetman <ddstreet@ieee.org>
> > Subject: mm/zsmalloc: don't fail if can't create debugfs info
> > 
> > Change the return type of zs_pool_stat_create() to void, and
> > remove the logic to abort pool creation if the stat debugfs
> > dir/file could not be created.
> > 
> > The debugfs stat file is for debugging/information only, and doesn't
> > affect operation of zsmalloc; there is no reason to abort creating
> > the pool if the stat file can't be created.  This was seen with
> > zswap, which used the same name for all pool creations, which caused
> > zsmalloc to fail to create a second pool for zswap if
> > CONFIG_ZSMALLOC_STAT was enabled.
> 
> no real objections from me. given that both zram and zswap now provide
> unique names for zsmalloc stats dir, this patch does not fix any "real"
> (observed) problem /* ENOMEM in debugfs_create_dir() is a different
> case */.  so it's more of a cosmetic patch.
> 

Logically, I agree with Dan that debugfs is just optional so it
shouldn't affect the module running *but* practically, debugfs_create_dir
failure with no memory would be rare. Rather than it, we would see
error from same entry naming like Dan's case.

If we removes such error propagation logic in case of same naming,
how do zsmalloc user can notice that debugfs entry was not created
although zs_creation was successful returns success?

Otherwise, future user of zsmalloc can miss it easily if they repeates
same mistakes. So, what's the gain with this patch in real practice?


> FWIW,
> Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> 
> 	-ss

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-28 15:36 Dan Streetman
2016-04-28 22:07 ` Andrew Morton
2016-04-29  0:38   ` Sergey Senozhatsky
2016-04-29  5:37     ` Minchan Kim [this message]
2016-04-29 14:50       ` Dan Streetman
2016-04-29 15:33         ` Minchan Kim
2016-05-03  2:18 ` Ganesh Mahendran
2016-05-19 15:18   ` [PATCHv2] " Dan Streetman
2016-05-20  2:33     ` Ganesh Mahendran
2016-05-20  4:08     ` Sergey Senozhatsky
2016-05-20 10:32       ` Dan Streetman
2016-05-23  3:03     ` 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=20160429053740.GA2431@bbox \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=dan.streetman@canonical.com \
    --cc=ddstreet@ieee.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ngupta@vflare.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=sjenning@redhat.com \
    --cc=yuzhao@google.com \
    /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

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git