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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.62.0705252137230.2657@pademelon.sonytel.be>
Date:	Fri, 25 May 2007 21:40:42 +0200 (CEST)
From:	Geert Uytterhoeven <Geert.Uytterhoeven@...ycom.com>
To:	Arnd Bergmann <arnd@...db.de>
cc:	linuxppc-dev@...abs.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 5/7] ps3: Disk Storage Driver

On Fri, 25 May 2007, Arnd Bergmann wrote:
> On Friday 25 May 2007, Geert.Uytterhoeven@...ycom.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@...ycom.com ------- The Corporate Village, Da Vincilaan 7-D1
Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/