On Wed, Nov 25, 2009 at 08:50:21PM +1030, Rusty Russell wrote:
> On Wed, 25 Nov 2009 07:45:30 pm Michael S. Tsirkin wrote:
> > Hmm, is it really worth it to save a header copy if it's linear? We are
> > going to access it anyway, and it fits into one cacheline nicely. On
> > the other hand we hav
On Wed, 25 Nov 2009 07:45:30 pm Michael S. Tsirkin wrote:
> Hmm, is it really worth it to save a header copy if it's linear? We are
> going to access it anyway, and it fits into one cacheline nicely. On
> the other hand we have more code making life harder for compiler and
> processor.
Not sure:
On Wed, Nov 25, 2009 at 10:42:06AM +1030, Rusty Russell wrote:
> On Tue, 24 Nov 2009 10:07:54 pm Michael S. Tsirkin wrote:
> > On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
> > > On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
> > > > > > + skb = (struct sk_buff *)buf
On Wed, 25 Nov 2009 01:06:32 am Anthony Liguori wrote:
> So does lguest. It's been that way since the beginning. Fixing this
> would result in breaking older guests.
Agreed, we can't "fix" it in the guests, but it's a wart. That's why I
haven't bothered fixing it, but if someone else wants to
On Tue, 24 Nov 2009 10:07:54 pm Michael S. Tsirkin wrote:
> On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
> > On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
> > > > > + skb = (struct sk_buff *)buf;
> > > > This cast is unnecessary, but a comment would be nice:
> > >
On Tue, Nov 24, 2009 at 08:36:32AM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
>>
>>> On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
>>>
>> + skb = (struct sk_buff *)buf;
>>
>>>
On Tue, Nov 24, 2009 at 08:36:32AM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
>>
>>> On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
>>>
>> + skb = (struct sk_buff *)buf;
>>
>>>
Michael S. Tsirkin wrote:
On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
+ skb = (struct sk_buff *)buf;
This cast is unnecessary, but a comment would be nice:
Without this cast there is a
On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
> On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
> > > > + skb = (struct sk_buff *)buf;
> > > This cast is unnecessary, but a comment would be nice:
> >
> > Without this cast there is a compile warning.
>
> Hi Shirley,
On Tue, 2009-11-24 at 08:54 +1030, Rusty Russell wrote:
> #define BIG_PACKET_PAD (NET_SKB_PAD - sizeof(struct virtio_net_hdr) +
> NET_IP_ALIGN)
> struct big_packet_page {
> struct virtio_net_hdr hdr;
> char pad[BIG_PACKET_PAD];
> /* Actual packet data starts here */
>
On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
> > > + skb = (struct sk_buff *)buf;
> > This cast is unnecessary, but a comment would be nice:
>
> Without this cast there is a compile warning.
Hi Shirley,
Looks like buf is a void *, so no cast should be necessary. But I could
Hello Rusty,
On Mon, 2009-11-23 at 11:38 +1030, Rusty Russell wrote:
> Overall, the patch looks good. But it would have been nicer if it
> were
> split into several parts: cleanups, new infrastructure, then the
> actual
> allocation change.
I have split the patch into a set: cleanups, new infras
On Mon, 2009-11-23 at 11:43 +0200, Michael S. Tsirkin wrote:
> should be !npage->private
> and nesting is too deep here:
> this is cleaner in a give_a_page subroutine
> as it was.
This will be addressed with Rusty's comment.
> > + /* use private to chain big packets */
>
> packets? o
On Mon, 2009-11-23 at 11:38 +1030, Rusty Russell wrote:
> How about:
> struct page *end;
>
> /* Find end of list, sew whole thing into vi->pages. */
> for (end = page; end->private; end = (struct page
> *)end->private);
> end->private = (unsigned long)vi->pages;
>
On Fri, Nov 20, 2009 at 08:21:41AM -0800, Shirley Ma wrote:
> On Fri, 2009-11-20 at 07:19 +0100, Eric Dumazet wrote:
> > Interesting use after free :)
>
> Thanks for catching the stupid mistake. This is the updated patch for
> review.
>
> Signed-off-by: Shirley Ma (x...@us.ibm.com)
some style co
On Sat, 21 Nov 2009 02:51:41 am Shirley Ma wrote:
> Signed-off-by: Shirley Ma (x...@us.ibm.com)
Hi Shirley,
This patch seems like a good idea, but it needs some work. As this is
the first time I've received a patch from you, I will go through it in extra
detail. (As virtio maintainer, it's m
On Fri, 2009-11-20 at 07:19 +0100, Eric Dumazet wrote:
> Interesting use after free :)
Thanks for catching the stupid mistake. This is the updated patch for
review.
Signed-off-by: Shirley Ma (x...@us.ibm.com)
--
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b9e002f..
On Fri, 2009-11-20 at 07:19 +0100, Eric Dumazet wrote:
> > +void virtio_free_pages(void *buf)
> > +{
> > + struct page *page = (struct page *)buf;
> > +
> > + while (page) {
> > + __free_pages(page, 0);
> > + page = (struct page *)page->private;
> > + }
> > +}
>
Shirley Ma a écrit :
> This patch is generated against 2.6 git tree. I didn't break up this
> patch since it has one functionality. Please review it.
>
> Thanks
> Shirley
>
> Signed-off-by: Shirley Ma
> --
>
> +void virtio_free_pages(void *buf)
> +{
> + struct page *page = (struct page
This patch is generated against 2.6 git tree. I didn't break up this
patch since it has one functionality. Please review it.
Thanks
Shirley
Signed-off-by: Shirley Ma
--
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b9e002f..6fb788b 100644
--- a/drivers/net/virtio_ne
20 matches
Mail list logo
