All of lore.kernel.org
 help / color / mirror / Atom feed
From: Toshi Kani <toshi.kani@hpe.com>
To: Ross Zwisler <ross.zwisler@linux.intel.com>,
	Jan Kara <jack@suse.cz>, Christoph Hellwig <hch@infradead.org>
Cc: "tytso@mit.edu" <tytso@mit.edu>,
	"linux-nvdimm@ml01.01.org" <linux-nvdimm@ml01.01.org>,
	"david@fromorbit.com" <david@fromorbit.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Parrish, Micah (HP Servers Linux R&D)" <micah.parrish@hpe.com>,
	"adilger.kernel@dilger.ca" <adilger.kernel@dilger.ca>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] ext4: Add alignment check for DAX mount
Date: Wed, 04 May 2016 09:42:23 -0600	[thread overview]
Message-ID: <1462376543.27137.71.camel@hpe.com> (raw)
In-Reply-To: <1462290096.27137.47.camel@hpe.com>

On Tue, 2016-05-03 at 15:41 +0000, Kani, Toshimitsu wrote:
> On Tue, 2016-05-03 at 08:43 -0600, Ross Zwisler wrote:
> > On Tue, May 03, 2016 at 11:00:21AM +0200, Jan Kara wrote:
> > > On Tue 03-05-16 01:44:10, Christoph Hellwig wrote:
> > > > 
> > > > Please come up with a version that doesn't require tons of
> > > > boilerplate code in every file system.
> > >
> > > Well, I was thinking about some helper as well but we could save ~4
> > > lines with that and that didn't seem significant to me. Most of the
> > > lines is actually reporting appropriate mount error in dmesg and that
> > > is fs-dependent so it needs to stay in the filesystem... So what do
> > > you have in mind?
> >
> > I guess if you wanted to reduce the code needed in each filesystem, you
> > could avoid having different error messages for each of the failure
> > conditions, and just print the error value.  All the error cases caught
> > by the current code are unique, so we aren't losing any
> > information.  The resulting patch for ext4 would look like this:
>
> I'd prefer to keep the "error: device does not support dax" and "error:
> unaligned partition for dax" messages since they clarify the problem as a
> user error. The former case is especially common with BTT. The "error:
> dax access failed (%d)" message is helpful when we need to look into the
> case.
> 
> If XFS is OK to use the messages similar to ext2/4, then we can do: 
> 
>  * Add a new helper function, say bdev_check_dax_mount(), which logs
> common error messages with pr_err(), such as:
>        "VFS (pmem0): error: device does not support dax"
>  * When bdev_check_dax_mount() returns with a negative value:
>    - XFS logs an additional message below via xfs_alert() and proceeds
> without dax option.
>        "XFS (pmem0): Turning DAX off."
>    - ext2/4 fails the mount.

Since v2 got multiple reviews, and Jan has added 2/3 into his tree
(thanks!), I will send "[PATCH v2-UPDATE 3/3]" which addresses Ross's
comments on v2 patch 3/3.  This keeps the v2 series.

Christoph, let me know if you'd still like to see v3 with the above changes
to share the same error messages among ext2/4 and xfs.

Thanks,
-Toshi

  reply	other threads:[~2016-05-04 15:42 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-02 18:42 [PATCH v2 0/3] Add alignment check for DAX mount Toshi Kani
2016-05-02 18:42 ` Toshi Kani
2016-05-02 18:42 ` Toshi Kani
2016-05-02 18:42 ` [PATCH v2 1/3] ext4: " Toshi Kani
2016-05-02 18:42   ` Toshi Kani
2016-05-02 18:42   ` Toshi Kani
2016-05-03  7:58   ` Jan Kara
2016-05-03  7:58     ` Jan Kara
2016-05-03  7:58     ` Jan Kara
2016-05-03  8:44   ` Christoph Hellwig
2016-05-03  9:00     ` Jan Kara
2016-05-03 14:43       ` Ross Zwisler
2016-05-03 15:41         ` Toshi Kani
2016-05-04 15:42           ` Toshi Kani [this message]
2016-05-02 18:42 ` [PATCH v2 2/3] ext2: " Toshi Kani
2016-05-02 18:42   ` Toshi Kani
2016-05-02 18:42   ` Toshi Kani
2016-05-03  8:01   ` Jan Kara
2016-05-03  8:01     ` Jan Kara
2016-05-03  8:01     ` Jan Kara
2016-05-02 18:42 ` [PATCH v2 3/3] xfs: " Toshi Kani
2016-05-02 18:42   ` Toshi Kani
2016-05-02 18:42   ` Toshi Kani
2016-05-02 19:50   ` Ross Zwisler
2016-05-02 19:50     ` Ross Zwisler
2016-05-02 19:50     ` Ross Zwisler
2016-05-02 19:54     ` Toshi Kani
2016-05-02 19:54       ` Toshi Kani
2016-05-02 19:54       ` Toshi Kani
2016-05-04 23:18   ` Dave Chinner
2016-05-04 23:18     ` Dave Chinner
2016-05-04 23:18     ` Dave Chinner
2016-05-04 23:41     ` Toshi Kani
2016-05-04 23:41       ` Toshi Kani
2016-05-04 23:41       ` Toshi Kani
2016-05-05  8:03       ` Jan Kara
2016-05-05  8:03         ` Jan Kara
2016-05-05  8:03         ` Jan Kara
2016-05-02 19:20 ` [PATCH v2 0/3] " Boaz Harrosh
2016-05-02 19:20   ` Boaz Harrosh
2016-05-02 19:20   ` Boaz Harrosh

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=1462376543.27137.71.camel@hpe.com \
    --to=toshi.kani@hpe.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@ml01.01.org \
    --cc=micah.parrish@hpe.com \
    --cc=ross.zwisler@linux.intel.com \
    --cc=tytso@mit.edu \
    /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.