From: Christoph Hellwig <hch@infradead.org>
To: Jan Stancek <jstancek@redhat.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>,
Naresh Kamboju <naresh.kamboju@linaro.org>,
LTP List <ltp@lists.linux.it>,
Linux-Next Mailing List <linux-next@vger.kernel.org>,
linux-fsdevel@vger.kernel.org, chrubis <chrubis@suse.cz>,
open list <linux-kernel@vger.kernel.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Mark Brown <broonie@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
lkft-triage@lists.linaro.org,
Christoph Hellwig <hch@infradead.org>,
linux-ext4 <linux-ext4@vger.kernel.org>,
Theodore Ts'o <tytso@mit.edu>
Subject: Re: LTP: diotest4.c:476: read to read-only space. returns 0: Success
Date: Mon, 11 Nov 2019 00:38:15 -0800 [thread overview]
Message-ID: <20191111083815.GA29540@infradead.org> (raw)
In-Reply-To: <1751469294.11431533.1573460380206.JavaMail.zimbra@redhat.com>
On Mon, Nov 11, 2019 at 03:19:40AM -0500, Jan Stancek wrote:
> > > loff_t length,
> > > if (pad)
> > > iomap_dio_zero(dio, iomap, pos, fs_block_size -
> > > pad);
> > > }
> > > - return copied ? copied : ret;
> > > + return copied ? (loff_t) copied : ret;
> >
> > I'm a little confused on this proposed fix -- why does casting size_t
> > (aka unsigned long) to loff_t (long long) on a 32-bit system change the
> > test outcome?
>
> Ternary operator has a return type and an attempt is made to convert
> each of operands to the type of the other. So, in this case "ret"
> appears to be converted to type of "copied" first. Both have size of
> 4 bytes on 32-bit x86:
Sounds like we should use a good old if here to avoid that whole problem
spacE:
if (copied)
return copied;
return ret;
> size_t copied = 0;
> int ret = -14;
> long long actor_ret = copied ? copied : ret;
>
> On x86_64: actor_ret == -14;
> On x86 : actor_ret == 4294967282
>
> > Does this same diotest4 failure happen with XFS? I ask
> > because XFS has been using iomap for directio for ages.
>
> Yes, it fails on XFS too.
Is this a new test? If not why was this never reported? Sounds like
we should add this test case to xfstests.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: ltp@lists.linux.it
Subject: [LTP] LTP: diotest4.c:476: read to read-only space. returns 0: Success
Date: Mon, 11 Nov 2019 00:38:15 -0800 [thread overview]
Message-ID: <20191111083815.GA29540@infradead.org> (raw)
In-Reply-To: <1751469294.11431533.1573460380206.JavaMail.zimbra@redhat.com>
On Mon, Nov 11, 2019 at 03:19:40AM -0500, Jan Stancek wrote:
> > > loff_t length,
> > > if (pad)
> > > iomap_dio_zero(dio, iomap, pos, fs_block_size -
> > > pad);
> > > }
> > > - return copied ? copied : ret;
> > > + return copied ? (loff_t) copied : ret;
> >
> > I'm a little confused on this proposed fix -- why does casting size_t
> > (aka unsigned long) to loff_t (long long) on a 32-bit system change the
> > test outcome?
>
> Ternary operator has a return type and an attempt is made to convert
> each of operands to the type of the other. So, in this case "ret"
> appears to be converted to type of "copied" first. Both have size of
> 4 bytes on 32-bit x86:
Sounds like we should use a good old if here to avoid that whole problem
spacE:
if (copied)
return copied;
return ret;
> size_t copied = 0;
> int ret = -14;
> long long actor_ret = copied ? copied : ret;
>
> On x86_64: actor_ret == -14;
> On x86 : actor_ret == 4294967282
>
> > Does this same diotest4 failure happen with XFS? I ask
> > because XFS has been using iomap for directio for ages.
>
> Yes, it fails on XFS too.
Is this a new test? If not why was this never reported? Sounds like
we should add this test case to xfstests.
next prev parent reply other threads:[~2019-11-11 8:38 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-07 13:59 LTP: diotest4.c:476: read to read-only space. returns 0: Success Naresh Kamboju
2019-11-07 13:59 ` [LTP] " Naresh Kamboju
2019-11-08 0:20 ` Jan Stancek
2019-11-08 0:20 ` [LTP] " Jan Stancek
2019-11-11 1:26 ` Darrick J. Wong
2019-11-11 1:26 ` [LTP] " Darrick J. Wong
2019-11-11 8:19 ` Jan Stancek
2019-11-11 8:19 ` [LTP] " Jan Stancek
2019-11-11 8:38 ` Christoph Hellwig [this message]
2019-11-11 8:38 ` Christoph Hellwig
2019-11-11 10:28 ` [PATCH] iomap: fix return value of iomap_dio_bio_actor on 32bit systems Jan Stancek
2019-11-11 10:28 ` [LTP] " Jan Stancek
2019-11-11 10:36 ` Christoph Hellwig
2019-11-11 10:36 ` [LTP] " Christoph Hellwig
2019-11-12 1:24 ` Darrick J. Wong
2019-11-12 1:24 ` [LTP] " Darrick J. Wong
2019-11-11 10:38 ` LTP: diotest4.c:476: read to read-only space. returns 0: Success Jan Stancek
2019-11-11 10:38 ` [LTP] " Jan Stancek
2019-11-11 18:26 ` Naresh Kamboju
2019-11-11 18:26 ` [LTP] " Naresh Kamboju
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=20191111083815.GA29540@infradead.org \
--to=hch@infradead.org \
--cc=arnd@arndb.de \
--cc=broonie@kernel.org \
--cc=chrubis@suse.cz \
--cc=darrick.wong@oracle.com \
--cc=jstancek@redhat.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-next@vger.kernel.org \
--cc=lkft-triage@lists.linaro.org \
--cc=ltp@lists.linux.it \
--cc=naresh.kamboju@linaro.org \
--cc=tytso@mit.edu \
--cc=viro@zeniv.linux.org.uk \
/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.