Re: [PATCH 1/2 v4] ahci add "em_buffer" attribute for AHCI hosts

[Harry Zhang <harry.zhang@amd.com>]

Hi Tejun,

On Thu, 2010-04-22 at 17:34 +0200, Tejun Heo wrote:
> Hello, Harry.
> 
> On 04/22/2010 12:15 PM, Harry Zhang wrote:
> > +	/* Since EM buffer is in ABAR, commonly, the buffer size should be
> > +	 * less than a page. Check buffer size against PAGE_SIZE in case of
> > +	 * some rare instance. Only transfer the first page in this case.
> > +	 */
> 
> Oh, the PAGE_SIZE limit comes from the way sysfs attributes are
> implemented.  The kernel buffer sysfs uses is PAGE_SIZE so
> reads/writes can't be larger than that.  If you write past PAGE_SIZE
> from show, you'll corrupt someone else's memory.
Yes, I know that. I just think the EM read buffer size should not larger
than the PAGE_SIZE in common, and thus, should not break the sysfs
attributes r/w buffer limitation. Anyway, I will shorten the comment.
> 
> > +	if (count > PAGE_SIZE) {
> > +		dev_printk(KERN_WARNING, dev, 
> > +			   "EM read buffer size %u is larger than %lu",
> > +			   hpriv->em_buf_sz, PAGE_SIZE);
> > +		count = PAGE_SIZE;
> > +	}
> 
> It probably would be better to use ata_port_printk() and
> printk_ratelimit() the message.
OK. I could not determine which is better. I think the EM buffer is
belong to the host rather than a port, so I chose the "dev_printk".
BTW, should this be a warning or an error?
> Thanks.
> 
Thanks,
Harry