Re: [PATCH 2/2] NET: Multiqueue network device support implementation.

2007-04-10 Thread Evgeniy Polyakov
On Mon, Apr 09, 2007 at 02:28:41PM -0700, Peter P Waskiewicz Jr ([EMAIL 
PROTECTED]) wrote:
 + alloc_size = (sizeof(struct net_device_subqueue) * queue_count);
 + 
 + p = kzalloc(alloc_size, GFP_KERNEL);
 + if (!p) {
 + printk(KERN_ERR alloc_netdev: Unable to allocate queues.\n);
 + return NULL;

I think you either do not want to print it, or want additional details
about device...

 + }
 + 
 + dev-egress_subqueue = p;
 + dev-egress_subqueue_count = queue_count;
 +
   dev-get_stats = maybe_internal_stats;
   setup(dev);
   strcpy(dev-name, name);
   return dev;
  }
 -EXPORT_SYMBOL(alloc_netdev);
 +EXPORT_SYMBOL(alloc_netdev_mq);
  
  /**
   *   free_netdev - free network device
 @@ -3345,6 +3358,7 @@ void free_netdev(struct net_device *dev)
  {
  #ifdef CONFIG_SYSFS
   /*  Compatibility with error handling in drivers */
 + kfree((char *)dev-egress_subqueue);
   if (dev-reg_state == NETREG_UNINITIALIZED) {
   kfree((char *)dev - dev-padded);
   return;
 @@ -3356,6 +3370,7 @@ void free_netdev(struct net_device *dev)
   /* will free via device release */
   put_device(dev-dev);
  #else
 + kfree((char *)dev-egress_subqueue);

Still casting :)


-- 
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] NET: Multiqueue network device support implementation.

2007-04-10 Thread Herbert Xu
Waskiewicz Jr, Peter P [EMAIL PROTECTED] wrote:

 @@ -3356,6 +3370,7 @@ void free_netdev(struct net_device *dev)
  /* will free via device release */
  put_device(dev-dev);
  #else
 +kfree((char *)dev-egress_subqueue);
  kfree((char *)dev - dev-padded);
  #endif
  }
 
 Ahem. Explain that cast.
 
This can be removed if needed; however, I'm just copying what
 the other kfree()'s are doing in this function.  Any instance of a
 typecast that I introduced in these patches are just following what
 others have done in that section of the code.  So the cast is just for
 consistency in this particular area.  If you'd like me to remove it, I
 can do that.

The other cast is there for the subtraction, not the kfree...

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/2] NET: Multiqueue network device support implementation.

2007-04-10 Thread Waskiewicz Jr, Peter P
 On Mon, Apr 09, 2007 at 02:28:41PM -0700, Peter P Waskiewicz 
 Jr ([EMAIL PROTECTED]) wrote:
  +   alloc_size = (sizeof(struct net_device_subqueue) * queue_count);
  + 
  +   p = kzalloc(alloc_size, GFP_KERNEL);
  +   if (!p) {
  +   printk(KERN_ERR alloc_netdev: Unable to 
 allocate queues.\n);
  +   return NULL;
 
 I think you either do not want to print it, or want 
 additional details about device...

Ok.  This is essentially the same output printed if the netdev itself
cannot be allocated.  Should I update both strings to have more
device-specific information?

 
  +   }
  + 
  +   dev-egress_subqueue = p;
  +   dev-egress_subqueue_count = queue_count;
  +
  dev-get_stats = maybe_internal_stats;
  setup(dev);
  strcpy(dev-name, name);
  return dev;
   }
  -EXPORT_SYMBOL(alloc_netdev);
  +EXPORT_SYMBOL(alloc_netdev_mq);
   
   /**
* free_netdev - free network device
  @@ -3345,6 +3358,7 @@ void free_netdev(struct net_device *dev)  {  
  #ifdef CONFIG_SYSFS
  /*  Compatibility with error handling in drivers */
  +   kfree((char *)dev-egress_subqueue);
  if (dev-reg_state == NETREG_UNINITIALIZED) {
  kfree((char *)dev - dev-padded);
  return;
  @@ -3356,6 +3370,7 @@ void free_netdev(struct net_device *dev)
  /* will free via device release */
  put_device(dev-dev);
   #else
  +   kfree((char *)dev-egress_subqueue);
 
 Still casting :)

The latest repost removes these casts.


Thanks for the feedback,

-PJ Waskiewicz
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] NET: Multiqueue network device support implementation.

2007-04-10 Thread Evgeniy Polyakov
On Tue, Apr 10, 2007 at 08:41:49AM -0700, Waskiewicz Jr, Peter P ([EMAIL 
PROTECTED]) wrote:
  On Mon, Apr 09, 2007 at 02:28:41PM -0700, Peter P Waskiewicz 
  Jr ([EMAIL PROTECTED]) wrote:
   + alloc_size = (sizeof(struct net_device_subqueue) * queue_count);
   + 
   + p = kzalloc(alloc_size, GFP_KERNEL);
   + if (!p) {
   + printk(KERN_ERR alloc_netdev: Unable to 
  allocate queues.\n);
   + return NULL;
  
  I think you either do not want to print it, or want 
  additional details about device...
 
 Ok.  This is essentially the same output printed if the netdev itself
 cannot be allocated.  Should I update both strings to have more
 device-specific information?

I wonder, if it is ever possible with gfp_kernel...

I think different patch would be ok.

-- 
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] NET: Multiqueue network device support implementation.

2007-04-09 Thread Jan Engelhardt
Hi,


On Apr 9 2007 14:28, Peter P Waskiewicz Jr wrote:
@@ -3345,6 +3358,7 @@ void free_netdev(struct net_device *dev)
 {
 #ifdef CONFIG_SYSFS
   /*  Compatibility with error handling in drivers */
+  kfree((char *)dev-egress_subqueue);
   if (dev-reg_state == NETREG_UNINITIALIZED) {
   kfree((char *)dev - dev-padded);
   return;
@@ -3356,6 +3370,7 @@ void free_netdev(struct net_device *dev)
   /* will free via device release */
   put_device(dev-dev);
 #else
+  kfree((char *)dev-egress_subqueue);
   kfree((char *)dev - dev-padded);
 #endif
 }

Ahem. Explain that cast.



Jan
-- 
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/2] NET: Multiqueue network device support implementation.

2007-04-09 Thread Waskiewicz Jr, Peter P
 Hi,
 
 
 On Apr 9 2007 14:28, Peter P Waskiewicz Jr wrote:
 @@ -3345,6 +3358,7 @@ void free_netdev(struct net_device *dev)  {  
 #ifdef CONFIG_SYSFS
  /*  Compatibility with error handling in drivers */
 +kfree((char *)dev-egress_subqueue);
  if (dev-reg_state == NETREG_UNINITIALIZED) {
  kfree((char *)dev - dev-padded);
  return;
 @@ -3356,6 +3370,7 @@ void free_netdev(struct net_device *dev)
  /* will free via device release */
  put_device(dev-dev);
  #else
 +kfree((char *)dev-egress_subqueue);
  kfree((char *)dev - dev-padded);
  #endif
  }
 
 Ahem. Explain that cast.

Jan,
This can be removed if needed; however, I'm just copying what
the other kfree()'s are doing in this function.  Any instance of a
typecast that I introduced in these patches are just following what
others have done in that section of the code.  So the cast is just for
consistency in this particular area.  If you'd like me to remove it, I
can do that.

Thanks,

-PJ Waskiewicz
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html