Re: [patch 5/7] ps3: Disk Storage Driver

[Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>]

On Fri, 25 May 2007, Arnd Bergmann wrote:
> On Friday 25 May 2007, Geert.Uytterhoeven@sonycom.com wrote:
> > +static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
> > +				   struct request *req, int gather)
> > +{
> > +	unsigned int sectors = 0, offset = 0;
> > +	struct bio *bio;
> > +	sector_t sector;
> > +	struct bio_vec *bvec;
> > +	unsigned int i = 0, j;
> > +	size_t size;
> > +	void *buf;
> > +
> > +	rq_for_each_bio(bio, req) {
> > +		sector = bio->bi_sector;
> > +		dev_dbg(&dev->sbd.core,
> > +			"%s:%u: bio %u: %u segs %u sectors from %lu\n",
> > +			__func__, __LINE__, i, bio_segments(bio),
> > +			bio_sectors(bio), sector);
> > +		bio_for_each_segment(bvec, bio, j) {
> > +			size = bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE;
> > +			buf = __bio_kmap_atomic(bio, j, KM_USER0);
> > +			if (gather)
> > +				memcpy(dev->bounce_buf+offset, buf, size);
> > +			else
> > +				memcpy(buf, dev->bounce_buf+offset, size);
> > +			offset += size;
> > +			__bio_kunmap_atomic(bio, KM_USER0);
> > +		}
> > +		sectors += bio_sectors(bio);
> > +		i++;
> > +	}
> > +}
> 
> So the hypervison uses guest-real addresses here? I would have expected
> it to use the kernel page tables, which lets you use vmap() to do
> scatter-gather.

Yes, it uses logical partion addresses, so we cannot create a virtually
contiguous mapping.

> > +static int ps3disk_thread(void *data)
> > +{
> > +	struct ps3_storage_device *dev = data;
> > +	struct ps3disk_private *priv = ps3disk_priv(dev);
> > +	request_queue_t *q = priv->queue;
> > +	struct request *req;
> > +
> > +	dev_dbg(&dev->sbd.core, "%s thread init\n", __func__);
> > +
> > +	current->flags |= PF_NOFREEZE;
> > +
> > +	while (!kthread_should_stop()) {
> > +		spin_lock_irq(&priv->lock);
> > +		set_current_state(TASK_INTERRUPTIBLE);
> > +		req = elv_next_request(q);
> > +		if (!req) {
> > +			spin_unlock_irq(&priv->lock);
> > +			schedule();
> > +			continue;
> > +		}
> > +		if (!blk_fs_request(req)) {
> > +			blk_dump_rq_flags(req, DEVICE_NAME " bad request");
> > +			end_request(req, 0);
> > +			spin_unlock_irq(&priv->lock);
> > +			continue;
> > +		}
> > +		spin_unlock_irq(&priv->lock);
> > +		ps3disk_handle_request_sg(dev, req);
> > +	}
> > +
> > +	dev_dbg(&dev->sbd.core, "%s thread exit\n", __func__);
> > +	return 0;
> > +}
> 
> I don't really understand what the kthread is needed for. You probably
> thought about multiple options and ended up with this, but having
> a comment in front of it might be helpful.

I used a kthread because the request function of a block device driver must be
non-blocking, and ps3stor_read_write_sectors() calls wait_for_completion().

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- Sony Network and Software Technology Center Europe (NSCE)
Geert.Uytterhoeven@sonycom.com ------- The Corporate Village, Da Vincilaan 7-D1
Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium