All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Monakhov <dmonakhov@openvz.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-kernel@vger.kernel.org, jens.axboe@oracle.com
Subject: Re: [PATCH 1/4] block: implement compatible DISCARD support
Date: Thu, 11 Feb 2010 16:45:13 +0300	[thread overview]
Message-ID: <87tytnga3a.fsf@openvz.org> (raw)
In-Reply-To: <20100211130945.GA7644@infradead.org> (Christoph Hellwig's message of "Thu, 11 Feb 2010 08:09:45 -0500")

Christoph Hellwig <hch@infradead.org> writes:

> On Thu, Feb 11, 2010 at 03:59:31PM +0300, Dmitry Monakhov wrote:
>> I mean that it is impossible to know was it really successful or not.
>> We may just replace this function with this following function.
>> const char* make_me_hapy()
>> {
>>    return "you are happy already.";
>> }
>
> Which is an entirely valid, although suboptimal implementation.
>
>> AFAIK Currently there is no any generic block interface which send
>> io requests without ability to check the result. 
>> The question is should it be sync or async it is not easy to design
>> simple async interface so let's use sync by default
>> BTW: That's why blkdev_issue_barrier has to wait by default. 
In fact wait is the only interface for issue_barrier.
> This is going to kill performance.
But it may be reasonable to allow caller to choose would it
wait and work fair, or to cheat in a name of performance.
>
>> > That's incorrect - both the scsi WRITE SAME and ATA UNMAP
>> > implementations write to the payload. 
>> WOW. What for?
>
> Becuase these commands contain ranges of to be flushed blocks
> in their payload.
Ok i've found. 
libata-scsi.c: ata_scsi_write_same_xlat
                  ata_set_lba_range_entries
It's was not obvious from the first glance. But it is the way how it
works for now. But seems what we still optimize things a bit
1) alloc page with GFP_HIGHUSER (because x86 arch still used)
2) Share page between eight bios.
>
>> > Which is a bit different from fixing efficiency issues in discard, I'd
>> > prefer that to be split into a separate patch, especially as there might
>> > be quite a bit of discussion on the zeroout behaviour.
>> Seems that you also right here. At list it is not obvious how we should
>> send compat_discard bios WRITE,WRITE_SYNC or WRITE_SYNC_PLUG?
>> But blkdev_issue_zeroout() interface allow all this flags.
>> let's wait a bit and i'll redesign the patchset correspondingly
>
> The !wait case is ansynchronous, so WRITE seems fine.  The wait and
> !barrier case is more interesting, as this one is very close to
> synchrous write semantics.  But I'm not sure it's really worth
> optimization for that, this case is not used for online discarding
> but things like mkfs that have the filesystem for itself, or that
> not yet merged background discard for xfs which doesn't care too
> much about I/O priority.

  reply	other threads:[~2010-02-11 13:45 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-11 10:53 Dmitry Monakhov
2010-02-11 10:57 ` [PATCH 2/4] block: support compat discard mode by default Dmitry Monakhov
2010-02-11 11:25   ` Dmitry Monakhov
2010-02-11 12:22   ` Christoph Hellwig
2010-02-11 10:59 ` [PATCH 3/4] ext4: convert extent zeroout to generic function Dmitry Monakhov
2010-02-11 10:59   ` Dmitry Monakhov
2010-02-11 11:01 ` [PATCH 4/4] btrfs: add discard_compat support Dmitry Monakhov
2010-02-11 11:15   ` Dmitry Monakhov
2010-02-11 12:21 ` [PATCH 1/4] block: implement compatible DISCARD support Christoph Hellwig
2010-02-11 12:59   ` Dmitry Monakhov
2010-02-11 13:09     ` Christoph Hellwig
2010-02-11 13:45       ` Dmitry Monakhov [this message]
2010-02-11 14:06         ` Christoph Hellwig
2010-02-11 14:40   ` Martin K. Petersen

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=87tytnga3a.fsf@openvz.org \
    --to=dmonakhov@openvz.org \
    --cc=hch@infradead.org \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH 1/4] block: implement compatible DISCARD support' \
    /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

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.