FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 09-11-2011, 02:50 PM
Christoph Hellwig
 
Default block: export __make_request

Avoid the hacks need for request based device mappers currently by simply
exporting the symbol instead of trying to get it through the back door.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/block/blk-core.c
================================================== =================
--- linux-2.6.orig/block/blk-core.c 2011-09-08 12:02:11.575274440 +0200
+++ linux-2.6/block/blk-core.c 2011-09-08 12:03:25.422774199 +0200
@@ -38,8 +38,6 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_r
EXPORT_TRACEPOINT_SYMBOL_GPL(block_rq_remap);
EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_complete);

-static int __make_request(struct request_queue *q, struct bio *bio);
-
/*
* For the allocated request tables
*/
@@ -1213,7 +1211,7 @@ void init_request_from_bio(struct reques
blk_rq_bio_prep(req->q, req, bio);
}

-static int __make_request(struct request_queue *q, struct bio *bio)
+int __make_request(struct request_queue *q, struct bio *bio)
{
const bool sync = !!(bio->bi_rw & REQ_SYNC);
struct blk_plug *plug;
@@ -1317,6 +1315,7 @@ out_unlock:
out:
return 0;
}
+EXPORT_SYMBOL_GPL(__make_request); /* for device mapper only */

/*
* If bio->bi_dev is a partition, remap the location
Index: linux-2.6/drivers/md/dm.c
================================================== =================
--- linux-2.6.orig/drivers/md/dm.c 2011-09-08 12:03:31.587273831 +0200
+++ linux-2.6/drivers/md/dm.c 2011-09-08 12:05:02.306774593 +0200
@@ -180,9 +180,6 @@ struct mapped_device {
/* forced geometry settings */
struct hd_geometry geometry;

- /* For saving the address of __make_request for request based dm */
- make_request_fn *saved_make_request_fn;
-
/* sysfs handle */
struct kobject kobj;

@@ -1420,13 +1417,6 @@ static int _dm_request(struct request_qu
return 0;
}

-static int dm_make_request(struct request_queue *q, struct bio *bio)
-{
- struct mapped_device *md = q->queuedata;
-
- return md->saved_make_request_fn(q, bio); /* call __make_request() */
-}
-
static int dm_request_based(struct mapped_device *md)
{
return blk_queue_stackable(md->queue);
@@ -1437,7 +1427,7 @@ static int dm_request(struct request_que
struct mapped_device *md = q->queuedata;

if (dm_request_based(md))
- return dm_make_request(q, bio);
+ return __make_request(q, bio);

return _dm_request(q, bio);
}
@@ -2172,7 +2162,6 @@ static int dm_init_request_based_queue(s
return 0;

md->queue = q;
- md->saved_make_request_fn = md->queue->make_request_fn;
dm_init_md_queue(md);
blk_queue_softirq_done(md->queue, dm_softirq_done);
blk_queue_prep_rq(md->queue, dm_prep_fn);
Index: linux-2.6/include/linux/blkdev.h
================================================== =================
--- linux-2.6.orig/include/linux/blkdev.h 2011-09-08 12:02:11.595273510 +0200
+++ linux-2.6/include/linux/blkdev.h 2011-09-08 12:02:33.038774679 +0200
@@ -680,6 +680,8 @@ extern int scsi_cmd_ioctl(struct request
extern int sg_scsi_ioctl(struct request_queue *, struct gendisk *, fmode_t,
struct scsi_ioctl_command __user *);

+extern int __make_request(struct request_queue *q, struct bio *bio);
+
/*
* A queue has just exitted congestion. Note this in the global counter of
* congested queues, and wake up anyone who was waiting for requests to be

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-12-2011, 09:59 AM
Jens Axboe
 
Default block: export __make_request

On 2011-09-11 16:50, Christoph Hellwig wrote:
> Avoid the hacks need for request based device mappers currently by simply
> exporting the symbol instead of trying to get it through the back door.

Good idea, lets rename it to blk_make_request() at the same time though.
I'll merge it as such.

--
Jens Axboe

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-12-2011, 12:25 PM
Christoph Hellwig
 
Default block: export __make_request

On Mon, Sep 12, 2011 at 11:59:12AM +0200, Jens Axboe wrote:
> On 2011-09-11 16:50, Christoph Hellwig wrote:
> > Avoid the hacks need for request based device mappers currently by simply
> > exporting the symbol instead of trying to get it through the back door.
>
> Good idea, lets rename it to blk_make_request() at the same time though.
> I'll merge it as such.

What happens to the existing blk_make_request?

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-12-2011, 12:26 PM
Jens Axboe
 
Default block: export __make_request

On 2011-09-12 14:25, Christoph Hellwig wrote:
> On Mon, Sep 12, 2011 at 11:59:12AM +0200, Jens Axboe wrote:
>> On 2011-09-11 16:50, Christoph Hellwig wrote:
>>> Avoid the hacks need for request based device mappers currently by simply
>>> exporting the symbol instead of trying to get it through the back door.
>>
>> Good idea, lets rename it to blk_make_request() at the same time though.
>> I'll merge it as such.
>
> What happens to the existing blk_make_request?

I ended up naming __make_request() blk_queue_bio() instead.

--
Jens Axboe

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-12-2011, 01:38 PM
Christoph Hellwig
 
Default block: export __make_request

On Mon, Sep 12, 2011 at 02:26:52PM +0200, Jens Axboe wrote:
> > What happens to the existing blk_make_request?
>
> I ended up naming __make_request() blk_queue_bio() instead.

I really hate naming things different from the method they are
implementing. I've tried to figure out what the point of the
old blk_make_request is - why would we not go through
generic_make_request for this?

Boaz, any idea?

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-12-2011, 06:04 PM
Jens Axboe
 
Default block: export __make_request

On 2011-09-12 15:38, Christoph Hellwig wrote:
> On Mon, Sep 12, 2011 at 02:26:52PM +0200, Jens Axboe wrote:
>>> What happens to the existing blk_make_request?
>>
>> I ended up naming __make_request() blk_queue_bio() instead.
>
> I really hate naming things different from the method they are
> implementing. I've tried to figure out what the point of the
> old blk_make_request is - why would we not go through
> generic_make_request for this?
>
> Boaz, any idea?

I tend to agree, we could rename the existing blk_make_request(). It
could be blk_make_request_from_bio() or something like that, since
that's what it does.

--
Jens Axboe

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-13-2011, 09:19 PM
Christoph Hellwig
 
Default block: export __make_request

On Mon, Sep 12, 2011 at 08:04:46PM +0200, Jens Axboe wrote:
> > I really hate naming things different from the method they are
> > implementing. I've tried to figure out what the point of the
> > old blk_make_request is - why would we not go through
> > generic_make_request for this?
> >
> > Boaz, any idea?
>
> I tend to agree, we could rename the existing blk_make_request(). It
> could be blk_make_request_from_bio() or something like that, since
> that's what it does.

It should at very least be renamed. But I still can't figure out what
it is for exactly.

There are three users:

(1) virtio_blk::virtblk_get_id():
This looks like it really should just use blk_rq_map_kern.
(2) osd_initiator::_make_request():
This one looks like it should just use the same scheme as
sg_io(), as it's doing the same thing.
(3) target_core_pscsi::__pscsi_map_SG():
Same as (2).


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-14-2011, 05:17 PM
Boaz Harrosh
 
Default block: export __make_request

On 9/14/2011 12:19 AM, Christoph Hellwig wrote:

On Mon, Sep 12, 2011 at 08:04:46PM +0200, Jens Axboe wrote:

I really hate naming things different from the method they are
implementing. I've tried to figure out what the point of the
old blk_make_request is - why would we not go through
generic_make_request for this?

Boaz, any idea?


I tend to agree, we could rename the existing blk_make_request(). It
could be blk_make_request_from_bio() or something like that, since
that's what it does.


It should at very least be renamed. But I still can't figure out what
it is for exactly.

There are three users:

(1) virtio_blk::virtblk_get_id():
This looks like it really should just use blk_rq_map_kern.
(2) osd_initiator::_make_request():
This one looks like it should just use the same scheme as
sg_io(), as it's doing the same thing.


Good god what sg_io? That broken pointr+length from user-mode that sg.c
and bsg.c are using? no can do it's not user-mode pointers, and it's not
pointer+length it's pages pointers of a bio. The only other structure
that could carry the same information is struct sg, but we work very
hard to get rid of this contraption. (scsi_execute_async or something
that it was)


blk_make_request() was made to be the parallel of __make_request, to be
used from filesystem level users. But with two differences.
1. Mainly support for none-FS BLOCK_PC requests
2. Also support chained bios. (was added later)


(3) target_core_pscsi::__pscsi_map_SG():
Same as (2).



There is no better suitable structure in current Kernel to carry a list
of pages, with optional offset and length, then bio struct. Given a bio
at hand. how do you make a block request out of it? (If it's not an
FS_PC type IO?)


As I remember target_core had their own pages-linked-list structure, and
how do you make a request out of that? again best at hand is bio.

bio is for a long time a page-pointers-carrier-structure and is out of
private block-level use. The filesystem level is full of it.


Thanks
Boaz

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-14-2011, 05:53 PM
Doug Dumitru
 
Default block: export __make_request

It would be great if this function were exported, but I would
encourage everyone to consider exporting it generally, not just for
GPL usage.

The kernel seems to be pretty clean that some subsystems are GPL only
(things like sysfs), and some are open (things like the device-mapper
interfaces). When a subsystem is part GPL and part open, it gets very
confusing for developers of commercial modules to know what to use and
what not to use.

The goal of GPL versus commercial exports should be to encourage GPL
code, not to punish commercial code by requiring stupid work arounds
that only waste time and hurt performance.

Again, most of the block IO layer is plain exports. Adding random GPL
exports seems somewhat arbitrary.

Then again, this is your code, and If you believe it should be GPL
linkable only, then that is fine. Just consider that there are
projects out there that are strange enough to be impacted and some
degree of symmetry is desirable.

As a side node, I would vote for longer, more descriptive names.
"__anything" should never be exported.

Doug Dumitru
EasyCo LLC



On Wed, Sep 14, 2011 at 10:17 AM, Boaz Harrosh <bharrosh@panasas.com> wrote:
> On 9/14/2011 12:19 AM, Christoph Hellwig wrote:
>>
>> On Mon, Sep 12, 2011 at 08:04:46PM +0200, Jens Axboe wrote:
>>>>
>>>> I really hate naming things different from the method they are
>>>> implementing. *I've tried to figure out what the point of the
>>>> old blk_make_request is - why would we not go through
>>>> generic_make_request for this?
>>>>
>>>> Boaz, any idea?
>>>
>>> I tend to agree, we could rename the existing blk_make_request(). It
>>> could be blk_make_request_from_bio() or something like that, since
>>> that's what it does.
>>
>> It should at very least be renamed. *But I still can't figure out what
>> it is for exactly.
>>
>> There are three users:
>>
>> *(1) virtio_blk::virtblk_get_id():
>> * * * *This looks like it really should just use blk_rq_map_kern.
>> *(2) osd_initiator::_make_request():
>> * * * *This one looks like it should just use the same scheme as
>> * * * *sg_io(), as it's doing the same thing.
>
> Good god what sg_io? That broken pointr+length from user-mode that sg.c
> and bsg.c are using? no can do it's not user-mode pointers, and it's not
> pointer+length it's pages pointers of a bio. The only other structure
> that could carry the same information is struct sg, but we work very hard to
> get rid of this contraption. (scsi_execute_async or something that it was)
>
> blk_make_request() was made to be the parallel of __make_request, to be
> used from filesystem level users. But with two differences.
> 1. Mainly support for none-FS BLOCK_PC requests
> 2. Also support chained bios. (was added later)
>
>> *(3) target_core_pscsi::__pscsi_map_SG():
>> * * * *Same as (2).
>>
>
> There is no better suitable structure in current Kernel to carry a list
> of pages, with optional offset and length, then bio struct. Given a bio
> at hand. how do you make a block request out of it? (If it's not an FS_PC
> type IO?)
>
> As I remember target_core had their own pages-linked-list structure, and
> how do you make a request out of that? again best at hand is bio.
>
> bio is for a long time a page-pointers-carrier-structure and is out of
> private block-level use. The filesystem level is full of it.
>
> Thanks
> Boaz
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at *http://vger.kernel.org/majordomo-info.html
>



--
Doug Dumitru
EasyCo LLC

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 09-14-2011, 06:40 PM
Alan Cox
 
Default block: export __make_request

On Wed, 14 Sep 2011 10:53:22 -0700
Doug Dumitru <doug@easyco.com> wrote:

> It would be great if this function were exported, but I would
> encourage everyone to consider exporting it generally, not just for
> GPL usage.

That would require a licensing change for the kernel. The kernel is a GPL
work. Some of us have not licensed it in any other way.

>
> The kernel seems to be pretty clean that some subsystems are GPL only
> (things like sysfs), and some are open

The _GPL is meant to make it clear they are internal symbols. The lack of
an _GPL does not make them usable by non GPL code, it merely indicates
that its perhaps more likely that you could in some cases shown something
was not a derivative work.

The licensing is determined by the license document, and that very
clearly says the boundary is derivative works. The rest is just vague
guidance.

In addition btw please be careful when trying to do this even with non
derivative code your lawyer is happy with. The kernel contains algorithms
which are subject to various US software patent mess where GPL use has
been granted, that may impact you if you use such code indirectly in non
GPL code.

Alan

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 10:56 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org