On 3/16/16 6:44 AM, Christoph Hellwig wrote:
> This patch implements two closely related changes: First it embedds a
> bio the ioend structure so that we don't have to allocate one separately.
> Second it uses the block layer bio chaining mechanism to chain additional
> bios off this first one if needed instead of manually accouting for
> multiple bio completions in the ioend structure. Together this removes a
> memory allocation per ioend and greatly simplifies the ioend setup and
> I/O completion path.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_aops.c | 247
> ++++++++++++++++++++++++-----------------------------
> fs/xfs/xfs_aops.h | 15 ++--
> fs/xfs/xfs_super.c | 26 ++----
> 3 files changed, 123 insertions(+), 165 deletions(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 5928770..42e7368 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -124,18 +124,25 @@ next_bh:
> */
> STATIC void
> xfs_destroy_ioend(
> - struct xfs_ioend *ioend)
> + struct xfs_ioend *ioend,
> + int error)
> {
> struct inode *inode = ioend->io_inode;
> - int error = ioend->io_error;
> + struct bio *last = ioend->io_bio;
> struct bio *bio, *next;
>
> - for (bio = ioend->io_bio_done; bio; bio = next) {
> + for (bio = &ioend->io_inline_bio; bio; bio = next) {
> struct bio_vec *bvec;
> int i;
>
> - next = bio->bi_private;
> - bio->bi_private = NULL;
> + /*
> + * For the last bio, bi_private points to the ioend, so we
> + * need to explicitly end the iteration here.
> + */
> + if (bio == last)
> + next = NULL;
> + else
> + next = bio->bi_private;
>
> /* walk each page on bio, ending page IO on them */
> bio_for_each_segment_all(bvec, bio, i)
> @@ -143,8 +150,6 @@ xfs_destroy_ioend(
>
> bio_put(bio);
Coverity thinks this is problematic, calling it a
"Free of address-of expression (BAD_FREE)"
CID 1362192
The issue is that if bio still == io_inline_bio, we are freeing
memory which was not allocated. Maybe this needs a:
if (bio != &ioend->io_inline_bio)
bio_put(bio);
or is there a better way?
thanks,
-Eric
|