3

I have a struct pointer array like this

   struct packets *p=(struct packets *)malloc(sizeof(struct packets)*30);

There are two steps of the program

Step 1) in producer thread I am getting network packets from RX-Ring (from Packet MMAP)

Step 2) and In consumer thread I am responding packets carrying syn=1,ack=0 received in step 1 using TX-Ring (Again Packet MMAP)

And algo s like this

Update -- I am really sorry I mis-wrote, real algorithm is like following

    main()
      pthread_create step1;// Detached thread // while loop inside
      pthread_create step2;//Detached thread  // another while loop inside
    end main

the struct packets *p is list of packets receiving and responding to IP packets. I heard about volatile objects that prevent compiler from optimization. the *p is allocated just once and I am using field inside struct packets to indicate the last operation performed on individual element of *p array (whether producer performed last operation or consumer performed last operation) from Google search of volatile objects I found following info

The volatile keyword is intended to prevent the compiler from applying any optimizations on objects that can change in ways that cannot be determined by the compiler. Objects declared as volatile are omitted from optimization because their values can be changed by code outside the scope of current code at any time.26-Apr-2020

So I like to know is it ok if I use struct packets *p pointer as volatile since values will frequently changed.

I am coding in C

And allocation of struct packets *p pointer array happens just once

Update

*p is global. and allocated in receiver just once like pthread_once

I am also using conditional variable to signal sending thread that packets are received

Sorry again

user786
  • 3,902
  • 4
  • 40
  • 72
  • I seriously doubt `volatile` is a silver bullet here. You need either atomics or a mutual exclusion section of protection. This looks, sounds, and smells, like a circular *queue*, and appropriate code should be implemented to manage said-same. The most common use for `volatile` is hardware change detection. A repeated read of, say, a hardware port mapped to some address location returns different values, though no code generated by the compiler, nor running in the process, makes those underlying changes. `volatile` is not a silver bullet for change recognition in multi-threaded programs. – WhozCraig Mar 15 '21 at 11:50
  • 99.5% of everything you find on the internet about `volatile` is either irrelevant or an outright falsehood. Here's the only thing about `volatile` you need to know: it works **only** in conjunction with memory-mapped hardware registers.Do you have a variable associated with a memory-mapped hardware register? Declare it `volatile`. Don't have one? Don't declare anything `volatile`. That's all there is to it. – n. m. could be an AI Mar 15 '21 at 11:50
  • `volatile` should be used if the data is accessed in different threads or in a signal handler and in normal processing. But the `volatile` keyword does not make sure that the different parts of your program get exclusive or atomic access to the data. It only tells the compiler that the value can change in ways unknown to the compiler or that access to the value may have side effects. – Bodo Mar 15 '21 at 11:52
  • You can ignore n.'pronouns'm.’s comment; it is in the 99.5%. – Eric Postpischil Mar 15 '21 at 11:52
  • Sorry, forgot about the signal handlers. You should declare variables accessed from signal handlers `volatile sig_atomic_t`. However declaring variables accessed from multiple threads `volatile` is completely useless. – n. m. could be an AI Mar 15 '21 at 11:56
  • Some documents I found initially that describe Packet MMAP seem to suggest a buffer for packets is created, and then the user program calls `poll` to wait for packets to arrive. This indicates the memory is changing without the knowledge of the compiler (the buffer is not modified by a `read` call that specifies its addresses or other method that would inform the compiler that, after calling `poll`, parts of the buffer might have been changed). But I would like to see higher-quality documentation for how the Packet MMAP interface operates. – Eric Postpischil Mar 15 '21 at 12:05
  • @EricPostpischil I have updated my question. I dont have any documentation of Packet MMAP that I found on the internet so guess I will try and see – user786 Mar 15 '21 at 12:18
  • 2
    @n.'pronouns'm. - FWIW - `volatile` works well for other things as well. Eg. I have used the `volatile` keyword to force my compiler to refrain from optimizing away a variable when testing algorithms, regardless of whether that variable has anything to do with memory mapped hardware. :) – ryyker Mar 15 '21 at 13:19
  • Every single time this question pops up, some confused PC programmers start ranting about re-entrancy, which is a completely different topic. It has absolutely nothing to do with "The volatile keyword is intended to prevent the compiler from applying any optimizations on objects that can change in ways that cannot be determined by the compiler." – Lundin Mar 15 '21 at 14:44
  • @Lundin I I have two questions1) if I am using volatile keyword 4 some `struct object pointer` with all the multithreading things like condition variables/Mutexes in application then is it useful 2 use volatile keyword on my pointer?&what do u mean by PC programmers?I also look into network drivers code like 2 develop one with allowing applications directly accessing character device file & maintaining `linkedList` of packets received from applications & directly writing those network packets 2 DMA buffer which is mapped with driver's NIC card RX/TX descriptors.guide me 4 both of these thing. – user786 Mar 15 '21 at 15:25
  • @ryyker please look at my above comment – user786 Mar 15 '21 at 15:25
  • @WhozCraig can u please explain a bit what is `hardware change detection` I don't get it – user786 Mar 15 '21 at 15:30
  • @Alex I suspect what it boils down to is how the data ends up in your struct. You can't typically use `struct` for raw data, because of padding, but also in this case since the network endianess is big endian and you are most likely on a little endian machine. So you have some manner of de-serialization routine, yes? And how does the data enter that one? If you DMA straight into C `struct` then you are very lucky if it happens to match. Anyway, DMA buffers should always be volatile for a different reason, namely to block caching. – Lundin Mar 15 '21 at 15:31
  • By "PC programmers" I mean the spoiled bunch who've gotten spoonfed by good compilers with system awareness, rather than those unfortunate among us who've written tons hardware-related code with interrupts with various more or less dysfunctional embedded systems compilers and therefore seen the missing volatile bug in first person far too many times. – Lundin Mar 15 '21 at 15:32
  • @n.'pronouns'm. `...-mapped hardware registers.Do you have a variable associated with a memory-mapped hardware register? Declare it volatile. Don't have one? Don't declare anything volatile. That's all there is to it` So if I am using it in driver then volatile is a must but if I am using in application then I can remove volatile from struct pointer. @Lundin said the same thing I can use it with DMA buffer/ – user786 Mar 15 '21 at 15:48
  • @ryyker You can do all kinds of outright illegal things for testing and especially debugging, volatile could well be one of them. – n. m. could be an AI Mar 15 '21 at 16:47
  • @Lundin Volatile works perfectly on embedded systems without out-of-order execution, memory reordering and who knows what else. Once you add these things to your hardware, volatile still works, but only say 99.95% of time. – n. m. could be an AI Mar 15 '21 at 16:53
  • DMA buffer? If you are writing a kernel driver, then maybe. You probably want to look it up in your kernel writing manual rather than applying random internet advice. – n. m. could be an AI Mar 15 '21 at 17:06
  • @Alex As a rule of thumb, I would always declare any piece of memory that is potentially accessed from multiple threads as `volatile`, even if you are using locking. Of course, this may not be strictly necessary on some platforms, but would you want to risk it? This is especially important in the case of ISRs and memory that is written to by DMA or other hardware peripherals. Of course, this is in addition to and *not instead of* using proper synchronization methods. – th33lf Mar 15 '21 at 18:44
  • 2
    This explains the issue from an embedded system's point of view: https://electronics.stackexchange.com/a/409570. Except it doesn't mention DMA buffers, which must obviously be volatile too. Anyone who argues has clearly never written a DMA driver... – Lundin Mar 15 '21 at 18:54

1 Answers1

2

Is the packet buffer being accessed by multiple threads? If not, you do not need to use the volatile keyword since the compiler can keep track of changes to this variable, and in fact, it is probably a good idea to allow the compiler to optimize normally. The fact that it 'frequently changes' is completely irrelevant.

On the other hand, if you are indeed accessing the buffer from multiple threads (or say, an interrupt service routine), or some other process modifies the data unbeknown to you, then you should declare the buffer as volatile, but also use something like a mutex or semaphore to prevent race conditions.

PS: After your update to the question, it definitely looks like the second case is true. Recommend to declare it as volatile, along with using proper synchronization.

(This answer that was shared in one of the comments has more detailed info on what problem volatile addresses.)

th33lf
  • 2,177
  • 11
  • 15
  • 1
    From a brief look at information on Packet MMAP, it seems to be an interface where the kernel makes network packets visible to the process. There might not be other threads within the process or other user processes in the system changing the data, but the operating system and hardware itself might be changing the data. Unfortunately, the documentation I have found initially is unclear about the interface. – Eric Postpischil Mar 15 '21 at 11:50
  • No, you should not declare any interprocess or interthread memory volatile. It is completely useless and only slows things down. A mutex is enough. – n. m. could be an AI Mar 15 '21 at 11:54
  • @n.'pronouns'm. C compilers are unaware of threading. If I busy-wait on a flag that is updated in another thread, it might optimize out the code completely. Of course a mutex can be used in lieu of the flag, but then the mutex internally probably contains something defined as `volatile`. Same difference. But I agree a mutex is indeed the better option. – th33lf Mar 15 '21 at 12:00
  • 1
    Threads are a part of the C standard since 2011. A mutex usually does not contain anything volatile, it contains a memory barrier. The fallacy about compiler optimisations is that the compiler is not the only thing that does optimisations. The CPU does optimisations of its own. You need a memory barrier to prevent those. The flag also needs to be atomic. – n. m. could be an AI Mar 15 '21 at 12:10
  • @n.'pronouns'm. I should have said 'many' C compilers. I admittedly did a bad job of explaining, but we are mixing two different concepts here. Volatile is not for locking or synchronization. It's purpose is to indicate to the compiler that this piece of memory may be accessed out of context. A mutex is used for a completely different purpose. Both come into play when multiple threads are involved - one is not a substitute for the other! – th33lf Mar 15 '21 at 12:21
  • I am using conditional variable/mutexs plus two threads that meant to get increase once I get this to work – user786 Mar 15 '21 at 12:22
  • 1
    "It's purpose is to indicate to the compiler that this piece of memory may be accessed out of context." The problem here is that if it *is* accessed out of context from another thread, you have a race condition. To stop race conditions, use a mutex or a similar synchronisation mechanism. Once you have a synchronisation mechanism in place, you **do not** need `volatile`. – n. m. could be an AI Mar 15 '21 at 12:31
  • 1
    @n.'pronouns'm. `volatile` in this context was never used to achieve re-entrancy. It is not a replacement for synchronization objects and nobody claimed it was either. The reason is, as quoted in the question, to prevent compilers from doing crazy optimizations when it can't determine that a variable might be updated because it was done so from inside a callback. This is a very well-known and very real issue on most embedded systems and also in some older hosted system. Modern hosted system compilers seem more callback-aware so you might not need to use it any longer on some compilers. – Lundin Mar 15 '21 at 14:48
  • 2
    Anyway, beating the dead horse... I wrote [this](https://stackoverflow.com/a/6996259/584518) 10 years ago after I got tired of the "you can't use volatile for synchronization" comments. Final sentence should still hold true: "in a multi-threaded program, the volatile keyword does not help with thread synchronization/safety, it does likely not act as a memory barrier, but it could prevent against dangerous assumptions by the compiler's optimizer." – Lundin Mar 15 '21 at 14:56
  • @Lundin Sorry I have no idea what you mean by re-entrancy. The claim was that volatile is needed in addition to synchronisation primitives. I'm saying that this claim is incorrect, at least when your compiler more or less adheres to the C standard. If it doesn't, then all sorts of crazy workarounds might be necessary. You call it a dangerous optimisation, I call it a compiler bug. – n. m. could be an AI Mar 15 '21 at 17:13
  • @n.'pronouns'm. C99 is also a C standard. And there are so many compilers out there that adhere to this standard and know absolutely nothing about mutexes or threading. – th33lf Mar 15 '21 at 18:26
  • @n.'pronouns'm. The issue discussed isn't even related to multi-threading, it's related to callback functions. – Lundin Mar 15 '21 at 18:48