Re: [dm-devel] [RFC PATCH 00/32] separate operations from flags in the bio/request structs
On Wed, Nov 11, 2015 at 01:53:24AM -0600, Mike Christie wrote: > We no longer have the bvec merge functions so the original reason given > in the thread/patch Bart referenced is no longer valid. > > Offlist it was suggested that dropping the argument from submit_bio > might still improve performance, but I modified xfs and dm and did some > testing and did not see anything. > > So the change is not needed, and it would only be done because people > feel it would improve the interface. I would defintively prefer the changed interface, and to me it also looks like it would make your overall patch simpler. If you disagree feel free to keep it as-is for now and I'll do another pass later. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 00/32] separate operations from flags in the bio/request structs
On Wed, Nov 11 2015 at 6:28am -0500, Christoph Hellwigwrote: > On Wed, Nov 11, 2015 at 01:53:24AM -0600, Mike Christie wrote: > > We no longer have the bvec merge functions so the original reason given > > in the thread/patch Bart referenced is no longer valid. > > > > Offlist it was suggested that dropping the argument from submit_bio > > might still improve performance, but I modified xfs and dm and did some > > testing and did not see anything. > > > > So the change is not needed, and it would only be done because people > > feel it would improve the interface. > > I would defintively prefer the changed interface, and to me it also > looks like it would make your overall patch simpler. If you disagree > feel free to keep it as-is for now and I'll do another pass later. The less churn the better. But honestly, not quite following the logic on why all this flag-day churn is needed. BUT if it needs to happen, because we'll soon hit a wall on supported operations via REQ_*, now is as good a time as any -- but best to limit the change to the flags IMHO. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [dm-devel] [RFC PATCH 00/32] separate operations from flags in the bio/request structs
On 11/07/2015 04:23 AM, Christoph Hellwig wrote: > On Wed, Nov 04, 2015 at 10:53:39AM -0600, Mike Christie wrote: >>> If you have to touch submit_bio() and submit_bio_wait(), how about >>> requiring the callers of these functions to set the cmd and flags >>> arguments in the bio structure and to leave out the cmd and flags >>> arguments from the submit_bio() and submit_bio_wait() functions ? A >>> (compile tested only) patch that implements this idea is available at >>> https://lkml.org/lkml/2014/6/2/173. >>> >> >> Yeah, I can do that. > > I think this would be useful to do at the beginning of the series. I just wanted to double check that we wanted to do this. We no longer have the bvec merge functions so the original reason given in the thread/patch Bart referenced is no longer valid. Offlist it was suggested that dropping the argument from submit_bio might still improve performance, but I modified xfs and dm and did some testing and did not see anything. So the change is not needed, and it would only be done because people feel it would improve the interface. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND RFC PATCH 00/32] separate operations from flags in the bio/request structs
On Wed, Nov 04, 2015 at 04:07:57PM -0600, mchri...@redhat.com wrote: > Known issues: > - REQ_FLUSH is still a flag, but should probably be a operation. > For lower level drivers like SCSI where we only get a flush, it makes > more sense to be a operation. However, upper layers like filesystems > can send down flushes with writes, so it is more of a flag for them. > I am still working on this. Actually it should be both. REQ_OP_FLUSH for a real flush operation, and a REQ_PREFLUSH bio flag that the request layer will sequence into an actual write an a flush operation. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [dm-devel] [RFC PATCH 00/32] separate operations from flags in the bio/request structs
On Wed, Nov 04, 2015 at 10:53:39AM -0600, Mike Christie wrote: > > If you have to touch submit_bio() and submit_bio_wait(), how about > > requiring the callers of these functions to set the cmd and flags > > arguments in the bio structure and to leave out the cmd and flags > > arguments from the submit_bio() and submit_bio_wait() functions ? A > > (compile tested only) patch that implements this idea is available at > > https://lkml.org/lkml/2014/6/2/173. > > > > Yeah, I can do that. I think this would be useful to do at the beginning of the series. While this will have a huge trickle effect through the series it should make thing a bit simpler overall. By leaving op 0 undefined it should also help to create a nice error trap for unconverted code. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND RFC PATCH 00/32] separate operations from flags in the bio/request structs
- Original Message - > This is just a resend of the patchset from earlier today. There was > a error in the middle of sending the set, so it looks like 10 - 32 got > dropped. > > There are a couple new block layer commands we are trying to add support > for in the near term: > > compare and write > http://www.spinics.net/lists/target-devel/msg07826.html > > copy offload/extended copy/xcopy > https://www.redhat.com/archives/dm-devel/2014-July/msg00070.html > > The problem is if we contine to add more commands we will have to one day > extend the cmd_flags/bi_rw fields again. To prevent that, this patchset > separates the operation (REQ_WRITE, REQ_DISCARD, REQ_WRITE_SAME, etc) from > the flags (REQ_SYNC, REQ_QUIET, etc) in the bio and request structs. In the > end of this set, we will have two fields bio->bi_op/request->op and > bio->bi_rw/request->cmd_flags. > > The patches were made against Jens's linux-block tree's for-linus branch: > https://git.kernel.org/cgit/linux/kernel/git/axboe/linux-block.git/log/?h=for-linus > (last commit a22c4d7e34402ccdf3414f64c50365436eba7b93). > > I have done some basic testing for a lot of the drivers and filesystems, > but I wanted to get comments before trying to track down more hardware/ > systems for testing. > > > Known issues: > - REQ_FLUSH is still a flag, but should probably be a operation. > For lower level drivers like SCSI where we only get a flush, it makes > more sense to be a operation. However, upper layers like filesystems > can send down flushes with writes, so it is more of a flag for them. > I am still working on this. > > - There is a regression with the dm flakey target. It currently > cannot corrupt the operation values. > > - The patchset is a little awkward. It touches so much code, > but I wanted to maintain git bisectibility, so there is lots of compat code > left around until the last patches where everyting is cleaned up. Hi Mike, The GFS2 patches in patches 11, 14, 16 and 23 all look correct. You can add my: Reviewed-by: Bob Petersonto those, but I have not reviewed the non-GFS2 bits. Regards, Bob Peterson Red Hat File Systems -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND RFC PATCH 00/32] separate operations from flags in the bio/request structs
This is just a resend of the patchset from earlier today. There was a error in the middle of sending the set, so it looks like 10 - 32 got dropped. There are a couple new block layer commands we are trying to add support for in the near term: compare and write http://www.spinics.net/lists/target-devel/msg07826.html copy offload/extended copy/xcopy https://www.redhat.com/archives/dm-devel/2014-July/msg00070.html The problem is if we contine to add more commands we will have to one day extend the cmd_flags/bi_rw fields again. To prevent that, this patchset separates the operation (REQ_WRITE, REQ_DISCARD, REQ_WRITE_SAME, etc) from the flags (REQ_SYNC, REQ_QUIET, etc) in the bio and request structs. In the end of this set, we will have two fields bio->bi_op/request->op and bio->bi_rw/request->cmd_flags. The patches were made against Jens's linux-block tree's for-linus branch: https://git.kernel.org/cgit/linux/kernel/git/axboe/linux-block.git/log/?h=for-linus (last commit a22c4d7e34402ccdf3414f64c50365436eba7b93). I have done some basic testing for a lot of the drivers and filesystems, but I wanted to get comments before trying to track down more hardware/ systems for testing. Known issues: - REQ_FLUSH is still a flag, but should probably be a operation. For lower level drivers like SCSI where we only get a flush, it makes more sense to be a operation. However, upper layers like filesystems can send down flushes with writes, so it is more of a flag for them. I am still working on this. - There is a regression with the dm flakey target. It currently cannot corrupt the operation values. - The patchset is a little awkward. It touches so much code, but I wanted to maintain git bisectibility, so there is lots of compat code left around until the last patches where everyting is cleaned up. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 00/32] separate operations from flags in the bio/request structs
There are a couple new block layer commands we are trying to add support for in the near term: compare and write http://www.spinics.net/lists/target-devel/msg07826.html copy offload/extended copy/xcopy https://www.redhat.com/archives/dm-devel/2014-July/msg00070.html The problem is if we contine to add more commands we will have to one day extend the cmd_flags/bi_rw fields again. To prevent that, this patchset separates the operation (REQ_WRITE, REQ_DISCARD, REQ_WRITE_SAME, etc) from the flags (REQ_SYNC, REQ_QUIET, etc) in the bio and request structs. In the end of this set, we will have two fields bio->bi_op/request->op and bio->bi_rw/request->cmd_flags. The patches were made against Jens's linux-block tree's for-linus branch: https://git.kernel.org/cgit/linux/kernel/git/axboe/linux-block.git/log/?h=for-linus (last commit a22c4d7e34402ccdf3414f64c50365436eba7b93). I have done some basic testing for a lot of the drivers and filesystems, but I wanted to get comments before trying to track down more hardware/ systems for testing. Known issues: - REQ_FLUSH is still a flag, but should probably be a operation. For lower level drivers like SCSI where we only get a flush, it makes more sense to be a operation. However, upper layers like filesystems can send down flushes with writes, so it is more of a flag for them. I am still working on this. - There is a regression with the dm flakey target. It currently cannot corrupt the operation values. - The patchset is a little awkward. It touches so much code, but I wanted to maintain git bisectibility, so there is lots of compat code left around until the last patches where everyting is cleaned up. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [dm-devel] [RFC PATCH 00/32] separate operations from flags in the bio/request structs
On 11/04/2015 08:32 AM, mchri...@redhat.com wrote: There are a couple new block layer commands we are trying to add support for in the near term: compare and write http://www.spinics.net/lists/target-devel/msg07826.html copy offload/extended copy/xcopy https://www.redhat.com/archives/dm-devel/2014-July/msg00070.html The problem is if we contine to add more commands we will have to one day extend the cmd_flags/bi_rw fields again. To prevent that, this patchset separates the operation (REQ_WRITE, REQ_DISCARD, REQ_WRITE_SAME, etc) from the flags (REQ_SYNC, REQ_QUIET, etc) in the bio and request structs. In the end of this set, we will have two fields bio->bi_op/request->op and bio->bi_rw/request->cmd_flags. The patches were made against Jens's linux-block tree's for-linus branch: https://git.kernel.org/cgit/linux/kernel/git/axboe/linux-block.git/log/?h=for-linus (last commit a22c4d7e34402ccdf3414f64c50365436eba7b93). I have done some basic testing for a lot of the drivers and filesystems, but I wanted to get comments before trying to track down more hardware/ systems for testing. Known issues: - REQ_FLUSH is still a flag, but should probably be a operation. For lower level drivers like SCSI where we only get a flush, it makes more sense to be a operation. However, upper layers like filesystems can send down flushes with writes, so it is more of a flag for them. I am still working on this. - There is a regression with the dm flakey target. It currently cannot corrupt the operation values. - The patchset is a little awkward. It touches so much code, but I wanted to maintain git bisectibility, so there is lots of compat code left around until the last patches where everyting is cleaned up. Hello Mike, If you have to touch submit_bio() and submit_bio_wait(), how about requiring the callers of these functions to set the cmd and flags arguments in the bio structure and to leave out the cmd and flags arguments from the submit_bio() and submit_bio_wait() functions ? A (compile tested only) patch that implements this idea is available at https://lkml.org/lkml/2014/6/2/173. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [dm-devel] [RFC PATCH 00/32] separate operations from flags in the bio/request structs
On 11/04/2015 10:49 AM, Bart Van Assche wrote: > Hello Mike, > > If you have to touch submit_bio() and submit_bio_wait(), how about > requiring the callers of these functions to set the cmd and flags > arguments in the bio structure and to leave out the cmd and flags > arguments from the submit_bio() and submit_bio_wait() functions ? A > (compile tested only) patch that implements this idea is available at > https://lkml.org/lkml/2014/6/2/173. > Yeah, I can do that. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html