Re: mbuf patch with sysctl suggestions too

On Tue, Jan 23, 2007 at 04:11:15PM -0500, Randall Stewart wrote:
Hi all:

Here is iteration 2 of the mbuf patch with limits I
proposed.

Also note the changes for sysctl stuff that Lugi suggested.
Please let me know what you think :-)

...
+ newnmbjclusters = nmbjumbop;
+ error = sysctl_int_checked(oidp, &newnmbjclusters, nmbjumbop,
+ SYSCTL_NO_LIMIT, req);

A few things here:
- i don't see much of a point in defining SYSCTL_NO_LIMIT;
UINT32_MAX would do perfectly there, and i think it is easier
to understand than SYSCTL_NO_LIMIT (which looks like a flag).

- here and in other places you do not allow decresaing the value
(by putting min = nmbjumbop etc.), and i am not sure why.
I understand a reasonable lower bound, but i guess the worst
that can happen, when you reduce the limit to something above
the current allocation, is that nothing is allocated until
you go again below the limit, right ?

- given that you don't use the new value if error != 0, perhaps
you can save the assignment newnmbjclusters = nmbjumbop;

And below:

+/*
+ * Handle an int, unsigned, but limited
+ * between min and max (unsigned)
+ * Two cases:
+ * a variable: point arg1 at it.
+ * a constant: pass it in arg2.
+ *
+ */
+
+extern int nmbjumbo9;
+
+int
+sysctl_int_checked(struct sysctl_oid *oidp, void *val, uint32_t min, uint32_t max, struct sysctl_req *req)
+{

the comment refers to something else and should be fixed e.g.

Handle an unsigned int variable with bound checking.
Returns 0 (and updates *val) if min <= v <= max;
returns EINVAL otherwhise (in which case *val is unchanged)

cheers
luigi
_______________________________________________
freebsd-net@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@xxxxxxxxxxx"

Relevant Pages

  • Re: mbuf patch with sysctl suggestions too
    ... Here is iteration 2 of the mbuf patch with limits I ... (by putting min = nmbjumbop etc.), and i am not sure why. ... the current allocation, is that nothing is allocated until ... + * Handle an int, unsigned, but limited ...
    (freebsd-net)
  • Re: [PATCH] Bring ext2 reservations code in line with latest ext3
    ... * If the reservation window is outside the goal allocation group, ... unsigned int group, struct super_block * sb) ... * is open for write (needs block allocation). ... void ext2_free_blocks (struct inode * inode, unsigned long block, ...
    (Linux-Kernel)
  • [PATCH] unified device list allocator
    ... reservations vs currently available devices so I tidied up the current ... Dynamic allocation is handled by passing in a start ... -static inline int major_to_index ... int unregister_blkdev(unsigned int major, const char *name) ...
    (Linux-Kernel)
  • Re: problem with memcpy and pointers/arrays confusion - again
    ... int line, unsigned long *total_mem); ... allocation for arbitrary types. ... Use a void *. ... void *mem; ...
    (comp.lang.c)
  • [PATCH 35/48] [GFS2] Allow bmap to allocate extents
    ... We've supported mapping of extents when no block allocation is required ... metadata tree in either height or depth) with the allocation of the data ... * @ip: The inode pointer ... unsigned int i; ...
    (Linux-Kernel)