On Fri, 25 May 2007, Arnd Bergmann wrote:

On Friday 25 May 2007, Geert.Uytterhoeven@sonycom.com wrote:

quoted

+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.

quoted

+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