Re: [RFC PATCH 02/20] ceph: add comments to metadata structures in buffer.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2025/09/05 22:00, Viacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
> Claude AI generated comments

Is LLM-generated text acceptable in the kernel?  Last week, I was
asked to add more explanations to a patch, but it was rejected because
I was accused of having used a LLM.

I feel that asking others to review LLM-generated content is a DoS on
their time.

>  /*
> - * a simple reference counted buffer.
> - *
> - * use kmalloc for smaller sizes, vmalloc for larger sizes.
> + * Reference counted buffer metadata: Simple buffer management with automatic
> + * memory allocation strategy. Uses kmalloc for smaller buffers and vmalloc
> + * for larger buffers to optimize memory usage and fragmentation.

This rephrasing done by your LLM is wrong.  Previously, the buffer
(i.e. the struct) was "simple".  Now, the management of this data
structure is called "simple".  Can you explain why you thought
changing this meaning is an improvement?

Even ignoring this part, I don't see any other improvement here.  This
just expands the wording (requires more time to read) but adds no
value.

>   */
>  struct ceph_buffer {
> +	/* Reference counting for safe shared access */

This is not "reference counting", but a "reference counter".  But who
really needs to be taught what a "struct kref" is?

And what does "safe shared access" mean?  This wording sounds like
it's for (thread) synchronization, but it's really only about knowing
when the object can be freed because no reference exists.

>  	struct kref kref;
> +	/* Kernel vector containing buffer pointer and length */

I don't think it is helpful to add an explanation of what a "struct
kvec" consists of here.  This is just redundant noise.

>  	struct kvec vec;
> +	/* Total allocated buffer size (may be larger than vec.iov_len) */
>  	size_t alloc_len;

This is the only useful piece of information added by this patch,
albeit trivial enough for everybody to induce from the name.

I'm not so excited about what you got from Claude here.




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux