diff mbox

[08/27] staging: lustre: avoid to use bio->bi_vcnt directly

Message ID 1459857443-20611-9-git-send-email-tom.leiming@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ming Lei April 5, 2016, 11:56 a.m. UTC
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/staging/lustre/lustre/llite/lloop.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Greg KH April 5, 2016, 12:59 p.m. UTC | #1
On Tue, Apr 05, 2016 at 07:56:53PM +0800, Ming Lei wrote:
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

A bit more of a commit message is always nice :)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig April 5, 2016, 1:01 p.m. UTC | #2
The lloop driver should be removed entirely - use the loop driver
instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Simmons April 10, 2016, 2:37 p.m. UTC | #3
> The lloop driver should be removed entirely - use the loop driver
> instead.

I talked with Andreas last week at our annual Lustre users group meeting 
about this. The reason I was told for existance is that some users were
using files on a Lustre file system with the loop back device. The 
performance was really bad at the time so a lloop was developed to 
overcome those limitations. Its been a long time so perhaps its time
to look at the default loop driver again to see if can perform now. If
it doesn't we will go the route of reworking the lloop driver in the
spirit of the cryptoloop device.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig April 10, 2016, 2:41 p.m. UTC | #4
On Sun, Apr 10, 2016 at 03:37:42PM +0100, James Simmons wrote:
> 
> > The lloop driver should be removed entirely - use the loop driver
> > instead.
> 
> I talked with Andreas last week at our annual Lustre users group meeting 
> about this. The reason I was told for existance is that some users were
> using files on a Lustre file system with the loop back device. The 
> performance was really bad at the time so a lloop was developed to 
> overcome those limitations. Its been a long time so perhaps its time
> to look at the default loop driver again to see if can perform now. If
> it doesn't we will go the route of reworking the lloop driver in the
> spirit of the cryptoloop device.

The loop driver now supports using AIO/DIO on any file systems that
implements ->read_iter and ->write_iter. If lustre doesn't support
those or doesn't have proper performance using them it should be
addressed in the file system.

Note that the dio mode in the loop device is not the default and you
need to manually enabled it, keep that in mind when testing.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Simmons April 10, 2016, 4:02 p.m. UTC | #5
> On Sun, Apr 10, 2016 at 03:37:42PM +0100, James Simmons wrote:
> > 
> > > The lloop driver should be removed entirely - use the loop driver
> > > instead.
> > 
> > I talked with Andreas last week at our annual Lustre users group meeting 
> > about this. The reason I was told for existance is that some users were
> > using files on a Lustre file system with the loop back device. The 
> > performance was really bad at the time so a lloop was developed to 
> > overcome those limitations. Its been a long time so perhaps its time
> > to look at the default loop driver again to see if can perform now. If
> > it doesn't we will go the route of reworking the lloop driver in the
> > spirit of the cryptoloop device.
> 
> The loop driver now supports using AIO/DIO on any file systems that
> implements ->read_iter and ->write_iter. If lustre doesn't support
> those or doesn't have proper performance using them it should be
> addressed in the file system.
> 
> Note that the dio mode in the loop device is not the default and you
> need to manually enabled it, keep that in mind when testing.

This is excellent news. The only sad thing is that most lustre users
are running distros that use kernels before the AIO/DIO enhancements
were landed :-( We will have to keep a copy around for those guys. But
first I need to test the performance of the loop back driver this
week before this can be dropped.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ming Lei April 11, 2016, 3:30 a.m. UTC | #6
On Mon, Apr 11, 2016 at 12:02 AM, James Simmons <jsimmons@infradead.org> wrote:
>
>> On Sun, Apr 10, 2016 at 03:37:42PM +0100, James Simmons wrote:
>> >
>> > > The lloop driver should be removed entirely - use the loop driver
>> > > instead.
>> >
>> > I talked with Andreas last week at our annual Lustre users group meeting
>> > about this. The reason I was told for existance is that some users were
>> > using files on a Lustre file system with the loop back device. The
>> > performance was really bad at the time so a lloop was developed to
>> > overcome those limitations. Its been a long time so perhaps its time
>> > to look at the default loop driver again to see if can perform now. If
>> > it doesn't we will go the route of reworking the lloop driver in the
>> > spirit of the cryptoloop device.
>>
>> The loop driver now supports using AIO/DIO on any file systems that
>> implements ->read_iter and ->write_iter. If lustre doesn't support
>> those or doesn't have proper performance using them it should be
>> addressed in the file system.
>>
>> Note that the dio mode in the loop device is not the default and you
>> need to manually enabled it, keep that in mind when testing.
>
> This is excellent news. The only sad thing is that most lustre users
> are running distros that use kernels before the AIO/DIO enhancements
> were landed :-( We will have to keep a copy around for those guys. But
> first I need to test the performance of the loop back driver this
> week before this can be dropped.

Considered that this cleanup patch for lustre loop is quite simple and
straightforward, I suggest to keep this cleanup patch as so and do the
dropping in another patchset. Christoph, are you OK with that?

Thanks,
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/staging/lustre/lustre/llite/lloop.c b/drivers/staging/lustre/lustre/llite/lloop.c
index b725fc1..67323db 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -302,19 +302,20 @@  static unsigned int loop_get_bio(struct lloop_device *lo, struct bio **req)
 	}
 
 	/* TODO: need to split the bio, too bad. */
-	LASSERT(first->bi_vcnt <= LLOOP_MAX_SEGMENTS);
+	LASSERT(bio_pages(first) <= LLOOP_MAX_SEGMENTS);
 
 	rw = first->bi_rw;
 	bio = &lo->lo_bio;
 	while (*bio && (*bio)->bi_rw == rw) {
+		unsigned curr_cnt = bio_pages(*bio);
 		CDEBUG(D_INFO, "bio sector %llu size %u count %u vcnt%u\n",
 		       (unsigned long long)(*bio)->bi_iter.bi_sector,
 		       (*bio)->bi_iter.bi_size,
-		       page_count, (*bio)->bi_vcnt);
-		if (page_count + (*bio)->bi_vcnt > LLOOP_MAX_SEGMENTS)
+		       page_count, curr_cnt);
+		if (page_count + curr_cnt > LLOOP_MAX_SEGMENTS)
 			break;
 
-		page_count += (*bio)->bi_vcnt;
+		page_count += curr_cnt;
 		count++;
 		bio = &(*bio)->bi_next;
 	}